Skip to content

v12.22.12 proposal #42531

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
richardlau merged 15 commits into from
Apr 5, 2022
Merged

v12.22.12 proposal #42531

Changes from 1 commit
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
a35f553
doc: add release key for Bryan English
bengl Feb 24, 2022
1193290
deps: V8: cherry-pick cc9a8a37445e
devsnek Mar 30, 2022
171bb66
node-api: force env shutdown deferring behavior
Feb 10, 2021
a2f4206
node-api: stop ref gc during environment teardown
gabrielschulhof Mar 4, 2021
2aa9ca1
node-api: fix crash in finalization
mhdawson Mar 23, 2021
81b4dc8
node-api: make reference weak parameter an indirect link to references
legendecas Mar 31, 2021
e707514
src: fix finalization crash
jasnell Apr 15, 2021
a7224c9
node-api: fix shutdown crashes
mhdawson Apr 30, 2021
e23c04f
node-api: avoid SecondPassCallback crash
mhdawson Jun 2, 2021
5f104e3
node-api: cctest on v8impl::Reference
legendecas Jun 8, 2021
a9c38f1
doc: add release key for Danielle Adams
danielleadams Oct 7, 2020
11aef2a
doc: update release key for Danielle Adams
danielleadams Dec 28, 2020
518a49c
doc: use openpgp.org for keyserver examples
nschonni Feb 18, 2022
333eda8
doc: add a note about possible missing lines to readline.asyncIterator
mikhalev-im Aug 8, 2020
a3d2837
2022-04-05, Version 12.22.12 'Erbium' (LTS)
richardlau Mar 30, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
node-api: avoid SecondPassCallback crash 
PR #38000 added
indirection so that we could stop finalization in
cases where it had been scheduled in a second
pass callback but we were doing it in advance in
environment teardown.

Unforunately we missed that the code which tries
to clear the second pass parameter checked if
the pointer to the parameter (_secondPassParameter)
was nullptr and that when the second pass callback
was scheduled we set _secondPassParameter to nullptr
in order to avoid it being deleted outside of the second
pass callback. The net result was that we
would not clear the _secondPassParameter contents
and failed to avoid the Finalization in the second pass
callback.

This PR adds an additional boolean for deciding if
the secondPassParameter should be deleted outside
of the second pass callback instead of setting
secondPassParameter to nullptr thus avoiding the
conflict between the 2 ways it was being used.

See the discussion starting at:
#38273 (comment)
for how this was discovered on OSX while trying to
upgrade to a new V8 version.

Signed-off-by: Michael Dawson <mdawson@devrus.com>

PR-URL: #38899
Backport-PR-URL: #42512
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
@mhdawson @richardlau
mhdawson authored and richardlau committed Mar 30, 2022
commit e23c04f0dcb4a7fbe40bfdac842f15147a819111
10 changes: 6 additions & 4 deletions src/js_native_api_v8.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,8 @@ class Reference : public RefBase {
Reference(napi_env env, v8::Local<v8::Value> value, Args&&... args)
: RefBase(env, std::forward<Args>(args)...),
_persistent(env->isolate, value),
_secondPassParameter(new SecondPassCallParameterRef(this)) {
_secondPassParameter(new SecondPassCallParameterRef(this)),
_secondPassScheduled(false) {
if (RefCount() == 0) {
SetWeak();
}
Expand All @@ -347,7 +348,7 @@ class Reference : public RefBase {
// If the second pass callback is scheduled, it will delete the
// parameter passed to it, otherwise it will never be scheduled
// and we need to delete it here.
if (_secondPassParameter != nullptr) {
if (!_secondPassScheduled) {
delete _secondPassParameter;
}
}
Expand Down Expand Up @@ -444,8 +445,7 @@ class Reference : public RefBase {
reference->_persistent.Reset();
// Mark the parameter not delete-able until the second pass callback is
// invoked.
reference->_secondPassParameter = nullptr;

reference->_secondPassScheduled = true;
data.SetSecondPassCallback(SecondPassCallback);
}

Expand All @@ -467,12 +467,14 @@ class Reference : public RefBase {
// the reference itself has already been deleted so nothing to do
return;
}
reference->_secondPassParameter = nullptr;
reference->Finalize();
}

bool env_teardown_finalize_started_ = false;
v8impl::Persistent<v8::Value> _persistent;
SecondPassCallParameterRef* _secondPassParameter;
bool _secondPassScheduled;
};

class ArrayBufferReference final : public Reference {
Expand Down