zlib: revert back to Functions by jasnell · Pull Request #13374 · nodejs/node

@nodejs-github-bot added the zlib

Issues and PRs related to the zlib subsystem.

label

Jun 1, 2017

addaleax

mscdex

watilde

@jasnell

This was referenced

Jun 1, 2017

mcollina

jasnell added a commit that referenced this pull request

Jun 5, 2017
Using ES6 Classes broke userland code. Revert back to functions.

PR-URL: #13374
Fixes: #13358
Ref: #13370
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

jasnell added a commit that referenced this pull request

Jun 5, 2017
Using ES6 Classes broke userland code. Revert back to functions.

PR-URL: #13374
Fixes: #13358
Ref: #13370
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

jasnell added a commit to jasnell/node that referenced this pull request

Jun 7, 2017

rvagg pushed a commit that referenced this pull request

Jun 8, 2017
* **Async Hooks**
  * When one `Promise` leads to the creation of a new `Promise`, the parent
    `Promise` will be identified as the trigger
    [[`135f4e6643`](135f4e6643)]
    [#13367](#13367).
* **Dependencies**
  * libuv has been updated to 1.12.0
    [[`968596ec77`](968596ec77)]
    [#13306](#13306).
  * npm has been updated to 5.0.3
    [[`ffa7debd7a`](ffa7debd7a)]
    [#13487](#13487).
* **File system**
  * The `fs.exists()` function now works correctly with `util.promisify()`
    [[`6e0eccd7a1`](6e0eccd7a1)]
    [#13316](#13316).
  * fs.Stats times are now also available as numbers
    [[`c756efb25a`](c756efb25a)]
    [#13173](#13173).
* **Inspector**
  * It is now possible to bind to a random port using `--inspect=0`
    [[`cc6ec2fb27`](cc6ec2fb27)]
    [#5025](#5025).
* **Zlib**
  * A regression in the Zlib module that made it impossible to properly
    subclasses `zlib.Deflate` and other Zlib classes has been fixed.
    [[`6aeb555cc4`](6aeb555cc4)]
    [#13374](#13374).

rvagg pushed a commit that referenced this pull request

Jun 8, 2017
* **Async Hooks**
  * When one `Promise` leads to the creation of a new `Promise`, the parent
    `Promise` will be identified as the trigger
    [[`135f4e6643`](135f4e6643)]
    [#13367](#13367).
* **Dependencies**
  * libuv has been updated to 1.12.0
    [[`968596ec77`](968596ec77)]
    [#13306](#13306).
  * npm has been updated to 5.0.3
    [[`ffa7debd7a`](ffa7debd7a)]
    [#13487](#13487).
* **File system**
  * The `fs.exists()` function now works correctly with `util.promisify()`
    [[`6e0eccd7a1`](6e0eccd7a1)]
    [#13316](#13316).
  * fs.Stats times are now also available as numbers
    [[`c756efb25a`](c756efb25a)]
    [#13173](#13173).
* **Inspector**
  * It is now possible to bind to a random port using `--inspect=0`
    [[`cc6ec2fb27`](cc6ec2fb27)]
    [#5025](#5025).
* **Zlib**
  * A regression in the Zlib module that made it impossible to properly
    subclasses `zlib.Deflate` and other Zlib classes has been fixed.
    [[`6aeb555cc4`](6aeb555cc4)]
    [#13374](#13374).

hhellyer pushed a commit to nodejs/llnode that referenced this pull request

Jun 29, 2017
* src: use explicit imports
Replace `using namespace lldb` with explicit `using lldb::<name>`
imports.

* test: fix scan-test.js with node >= 8.1.0
The object change that commit b73e042 ("src,test: support node.js >= 8")
from April addressed has been reverted again in 8.1.0.  Update the test.
Refs: nodejs/node#13374

* src: print builtins and unnamed stack frames
Previously, `v8 bt` would exclude frames that didn't map to a C++ symbol
or a JS stack frame.  llnode does not currently know how to identify the
stack frames of V8 builtins so those were omitted as well.

This commit makes those stack frames visible and introduces a heuristic
(in lldb >= 3.9) where frames whose PC is inside a WX memory segment are
assumed to belong to V8 builtins.

Fixes: #99
* fixup! SBMemoryRegionInfo is lldb >= 3.9

Fix: #99
PR-URL: #104
Reviewed-By: Fedor Indutny <fedor@indutny.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

danbev added a commit to danbev/node that referenced this pull request

Jan 7, 2021
This commit introduces a mutex field on the ManagedEVPPKey class
intended to be used when multiple threads require access to an OpenSSL
EVP_PKEY object. The motivation for this came from the work being done
to upgrade Node.js to OpenSSL 3.0.

OpenSSL objects, like EVP_PKEY, are not thread safe (see refs for
details). In versions prior to OpenSSL 3.0 this was not noticeable and
did not cause any issues (like incorrect logic or crashes), but with
OpenSSL 3.0 this does cause issues if access to an EVP_PKEY instance is
required from multiple threads without locking.

In OpenSSL 3.0 when the evp_pkey_downgrade function is called, which
downgrades an EVP_PKEY instance to a legacy version, it will clear all
the fields of EVP_PKEY struct except the lock (nodejs#13374). But this also
means that keymgmt and keydata will also be cleared, which other parts
of the code base depends on, and those calls will either fail to export
the key (returning null) or crash due to a segment fault.

This same code works with OpenSSL 1.1.1 without locking and I think this
is because there is no downgrade being done in OpenSSL 1.1.1. But even
so, as far as I can tell there are no guarantees that these object are
thread safe in 1.1.1 either and should be protected with a lock.

Refs:
openssl/openssl#13374
openssl/openssl#2165)
https://www.openssl.org/blog/blog/2017/02/21/threads

danbev added a commit that referenced this pull request

Jan 11, 2021
This commit introduces a Mutex field on the ManagedEVPPKey class
intended to be used when multiple threads require access to an OpenSSL
EVP_PKEY object.

The motivation for this is that OpenSSL objects like EVP_PKEY are not
thread safe. In versions prior to OpenSSL 3.0 this was not noticed and
did not cause any issues, like incorrect logic or crashes, but with
OpenSSL 3.0 this does cause problems if access to an EVP_PKEY instance
is required from multiple threads without locking.

In OpenSSL 3.0 where the evp_pkey_downgrade function is called, which
downgrades an EVP_PKEY instance to a legacy version, it will clear all
the fields of EVP_PKEY struct except the lock (#13374). But this will
also means that keymgmt and keydata will also be cleared which other
parts of the code base depends upon, and those calls will either fail
to export the key (returning null) or crash due to a segment fault.
This same code works with OpenSSL 1.1.1 and I'm guessing this is
because there is no downgrade in OpenSSL 1.1.1 (there is only the now
legacy struct) and the above situation never happens.

Refs:
openssl/openssl#13374
openssl/openssl#2165)
https://www.openssl.org/blog/blog/2017/02/21/threads

danbev added a commit that referenced this pull request

Jan 12, 2021
This commit introduces a Mutex field on the ManagedEVPPKey class
intended to be used when multiple threads require access to an OpenSSL
EVP_PKEY object.

The motivation for this is that OpenSSL objects like EVP_PKEY are not
thread safe. In versions prior to OpenSSL 3.0 this was not noticed and
did not cause any issues, like incorrect logic or crashes, but with
OpenSSL 3.0 this does cause problems if access to an EVP_PKEY instance
is required from multiple threads without locking.

In OpenSSL 3.0 where the evp_pkey_downgrade function is called, which
downgrades an EVP_PKEY instance to a legacy version, it will clear all
the fields of EVP_PKEY struct except the lock (#13374). But this will
also means that keymgmt and keydata will also be cleared which other
parts of the code base depends upon, and those calls will either fail
to export the key (returning null) or crash due to a segment fault.
This same code works with OpenSSL 1.1.1 and I'm guessing this is
because there is no downgrade in OpenSSL 1.1.1 (there is only the now
legacy struct) and the above situation never happens.

Refs:
openssl/openssl#13374
openssl/openssl#2165)
https://www.openssl.org/blog/blog/2017/02/21/threads

danbev added a commit to danbev/node that referenced this pull request

Jan 21, 2021
This commit introduces a mutex field on the ManagedEVPPKey class
intended to be used when multiple threads require access to an OpenSSL
EVP_PKEY object. The motivation for this came from the work being done
to upgrade Node.js to OpenSSL 3.0.

OpenSSL objects, like EVP_PKEY, are not thread safe (see refs for
details). In versions prior to OpenSSL 3.0 this was not noticeable and
did not cause any issues (like incorrect logic or crashes), but with
OpenSSL 3.0 this does cause issues if access to an EVP_PKEY instance is
required from multiple threads without locking.

In OpenSSL 3.0 when the evp_pkey_downgrade function is called, which
downgrades an EVP_PKEY instance to a legacy version, it will clear all
the fields of EVP_PKEY struct except the lock (nodejs#13374). But this also
means that keymgmt and keydata will also be cleared, which other parts
of the code base depends on, and those calls will either fail to export
the key (returning null) or crash due to a segment fault.

This same code works with OpenSSL 1.1.1 without locking and I think this
is because there is no downgrade being done in OpenSSL 1.1.1. But even
so, as far as I can tell there are no guarantees that these object are
thread safe in 1.1.1 either and should be protected with a lock.

Refs:
openssl/openssl#13374
openssl/openssl#2165)
https://www.openssl.org/blog/blog/2017/02/21/threads

danbev added a commit to danbev/node that referenced this pull request

Feb 9, 2021
This commit introduces a mutex field on the ManagedEVPPKey class
intended to be used when multiple threads require access to an OpenSSL
EVP_PKEY object. The motivation for this came from the work being done
to upgrade Node.js to OpenSSL 3.0.

OpenSSL objects, like EVP_PKEY, are not thread safe (see refs for
details). In versions prior to OpenSSL 3.0 this was not noticeable and
did not cause any issues (like incorrect logic or crashes), but with
OpenSSL 3.0 this does cause issues if access to an EVP_PKEY instance is
required from multiple threads without locking.

In OpenSSL 3.0 when the evp_pkey_downgrade function is called, which
downgrades an EVP_PKEY instance to a legacy version, it will clear all
the fields of EVP_PKEY struct except the lock (nodejs#13374). But this also
means that keymgmt and keydata will also be cleared, which other parts
of the code base depends on, and those calls will either fail to export
the key (returning null) or crash due to a segment fault.

This same code works with OpenSSL 1.1.1 without locking and I think this
is because there is no downgrade being done in OpenSSL 1.1.1. But even
so, as far as I can tell there are no guarantees that these object are
thread safe in 1.1.1 either and should be protected with a lock.

Refs:
openssl/openssl#13374
openssl/openssl#2165)
https://www.openssl.org/blog/blog/2017/02/21/threads

danbev added a commit that referenced this pull request

Feb 10, 2021
This commit introduces a mutex field on the ManagedEVPPKey class
intended to be used when multiple threads require access to an OpenSSL
EVP_PKEY object. The motivation for this came from the work being done
to upgrade Node.js to OpenSSL 3.0.

OpenSSL objects, like EVP_PKEY, are not thread safe (see refs for
details). In versions prior to OpenSSL 3.0 this was not noticeable and
did not cause any issues (like incorrect logic or crashes), but with
OpenSSL 3.0 this does cause issues if access to an EVP_PKEY instance is
required from multiple threads without locking.

In OpenSSL 3.0 when the evp_pkey_downgrade function is called, which
downgrades an EVP_PKEY instance to a legacy version, it will clear all
the fields of EVP_PKEY struct except the lock (#13374). But this also
means that keymgmt and keydata will also be cleared, which other parts
of the code base depends on, and those calls will either fail to export
the key (returning null) or crash due to a segment fault.

This same code works with OpenSSL 1.1.1 without locking and I think this
is because there is no downgrade being done in OpenSSL 1.1.1. But even
so, as far as I can tell there are no guarantees that these object are
thread safe in 1.1.1 either and should be protected with a lock.

PR-URL: #36825
Refs: openssl/openssl#13374
Refs: openssl/openssl#13374
Refs: openssl/openssl#2165)
Refs: https://www.openssl.org/blog/blog/2017/02/21/threads
Reviewed-By: James M Snell <jasnell@gmail.com>

danielleadams pushed a commit that referenced this pull request

Feb 16, 2021
This commit introduces a mutex field on the ManagedEVPPKey class
intended to be used when multiple threads require access to an OpenSSL
EVP_PKEY object. The motivation for this came from the work being done
to upgrade Node.js to OpenSSL 3.0.

OpenSSL objects, like EVP_PKEY, are not thread safe (see refs for
details). In versions prior to OpenSSL 3.0 this was not noticeable and
did not cause any issues (like incorrect logic or crashes), but with
OpenSSL 3.0 this does cause issues if access to an EVP_PKEY instance is
required from multiple threads without locking.

In OpenSSL 3.0 when the evp_pkey_downgrade function is called, which
downgrades an EVP_PKEY instance to a legacy version, it will clear all
the fields of EVP_PKEY struct except the lock (#13374). But this also
means that keymgmt and keydata will also be cleared, which other parts
of the code base depends on, and those calls will either fail to export
the key (returning null) or crash due to a segment fault.

This same code works with OpenSSL 1.1.1 without locking and I think this
is because there is no downgrade being done in OpenSSL 1.1.1. But even
so, as far as I can tell there are no guarantees that these object are
thread safe in 1.1.1 either and should be protected with a lock.

PR-URL: #36825
Refs: openssl/openssl#13374
Refs: openssl/openssl#13374
Refs: openssl/openssl#2165)
Refs: https://www.openssl.org/blog/blog/2017/02/21/threads
Reviewed-By: James M Snell <jasnell@gmail.com>