Skip to content

Commit

Permalink
src: fix crash on FSReqPromise destructor
Browse files Browse the repository at this point in the history
We are deciding whether to end `fs` promises by checking
`can_call_into_js()` whereas in the `FSReqPromise` destructor we're
using the `is_stopping()` check. Though this may look as semantically
correct it has issues because though both values are modified before
termination on `Environment::ExitEnv()` and both are atomic they are not
syncronized together so it may happen that when reaching the destructor
`call_into_js` may be set to `false` whereas `is_stopping` remains
`false` causing the crash. Fix this by checking with
`can_call_into_js()` also in the destructor.

Fixes: nodejs/node#43499

PR-URL: nodejs/node#43533
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
santigimeno authored and guangwong committed Oct 10, 2022
1 parent 19b6d05 commit cbe26cc
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion src/node_file-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ FSReqPromise<AliasedBufferT>::~FSReqPromise() {
// Validate that the promise was explicitly resolved or rejected but only if
// the Isolate is not terminating because in this case the promise might have
// not finished.
if (!env()->is_stopping()) CHECK(finished_);
CHECK_IMPLIES(!finished_, !env()->can_call_into_js());
}

template <typename AliasedBufferT>
Expand Down

0 comments on commit cbe26cc

Please sign in to comment.