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 using the same checks everywhere.

Fixes: nodejs#43499
  • Loading branch information
santigimeno committed Jun 22, 2022
1 parent 49dc7b7 commit f3863bb
Showing 1 changed file with 2 additions and 2 deletions.
4 changes: 2 additions & 2 deletions src/node_file.cc
Original file line number Diff line number Diff line change
Expand Up @@ -377,7 +377,7 @@ MaybeLocal<Promise> FileHandle::ClosePromise() {
std::unique_ptr<CloseReq> close(CloseReq::from_req(req));
CHECK_NOT_NULL(close);
close->file_handle()->AfterClose();
if (!close->env()->can_call_into_js()) return;
if (close->env()->is_stopping()) return;
Isolate* isolate = close->env()->isolate();
if (req->result < 0) {
HandleScope handle_scope(isolate);
Expand Down Expand Up @@ -651,7 +651,7 @@ void FSReqAfterScope::Reject(uv_fs_t* req) {
}

bool FSReqAfterScope::Proceed() {
if (!wrap_->env()->can_call_into_js()) {
if (wrap_->env()->is_stopping()) {
return false;
}

Expand Down

0 comments on commit f3863bb

Please sign in to comment.