diff --git a/src/node_dir.cc b/src/node_dir.cc index 9923f042779f2e..92d6bc96e48ad5 100644 --- a/src/node_dir.cc +++ b/src/node_dir.cc @@ -197,8 +197,8 @@ static MaybeLocal DirentListToArray( } static void AfterDirRead(uv_fs_t* req) { - FSReqBase* req_wrap = FSReqBase::from_req(req); - FSReqAfterScope after(req_wrap, req); + BaseObjectPtr req_wrap { FSReqBase::from_req(req) }; + FSReqAfterScope after(req_wrap.get(), req); if (!after.Proceed()) { return; @@ -210,12 +210,12 @@ static void AfterDirRead(uv_fs_t* req) { if (req->result == 0) { // Done Local done = Null(isolate); + after.Clear(); req_wrap->Resolve(done); return; } uv_dir_t* dir = static_cast(req->ptr); - req->ptr = nullptr; Local error; Local js_array; @@ -224,9 +224,13 @@ static void AfterDirRead(uv_fs_t* req) { req->result, req_wrap->encoding(), &error).ToLocal(&js_array)) { + // Clear libuv resources *before* delivering results to JS land because + // that can schedule another operation on the same uv_dir_t. Ditto below. + after.Clear(); return req_wrap->Reject(error); } + after.Clear(); req_wrap->Resolve(js_array); } diff --git a/src/node_file.cc b/src/node_file.cc index d0d5cdc1fe9f83..bc99eaa612d37d 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -540,8 +540,15 @@ FSReqAfterScope::FSReqAfterScope(FSReqBase* wrap, uv_fs_t* req) } FSReqAfterScope::~FSReqAfterScope() { + Clear(); +} + +void FSReqAfterScope::Clear() { + if (!wrap_) return; + uv_fs_req_cleanup(wrap_->req()); - delete wrap_; + wrap_->Detach(); + wrap_.reset(); } // TODO(joyeecheung): create a normal context object, and @@ -554,12 +561,16 @@ FSReqAfterScope::~FSReqAfterScope() { // which is also why the errors should have been constructed // in JS for more flexibility. void FSReqAfterScope::Reject(uv_fs_t* req) { - wrap_->Reject(UVException(wrap_->env()->isolate(), - req->result, - wrap_->syscall(), - nullptr, - req->path, - wrap_->data())); + BaseObjectPtr wrap { wrap_ }; + Local exception = + UVException(wrap_->env()->isolate(), + req->result, + wrap_->syscall(), + nullptr, + req->path, + wrap_->data()); + Clear(); + wrap->Reject(exception); } bool FSReqAfterScope::Proceed() { diff --git a/src/node_file.h b/src/node_file.h index 1ebfe06c1fbdbf..20621af528ea5a 100644 --- a/src/node_file.h +++ b/src/node_file.h @@ -157,6 +157,7 @@ class FSReqAfterScope final { public: FSReqAfterScope(FSReqBase* wrap, uv_fs_t* req); ~FSReqAfterScope(); + void Clear(); bool Proceed(); @@ -168,7 +169,7 @@ class FSReqAfterScope final { FSReqAfterScope& operator=(const FSReqAfterScope&&) = delete; private: - FSReqBase* wrap_ = nullptr; + BaseObjectPtr wrap_; uv_fs_t* req_ = nullptr; v8::HandleScope handle_scope_; v8::Context::Scope context_scope_;