Skip to content

Commit

Permalink
fs: check closing_ in FileHandle::Close
Browse files Browse the repository at this point in the history
Fix possible flaky failure. Keep uv_fs_close from being called twice
on the same fd.

Signed-off-by: James M Snell <jasnell@gmail.com>

PR-URL: #39472
Refs: #39464
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
  • Loading branch information
jasnell authored and targos committed Jul 25, 2021
1 parent aa2d5f9 commit a04f9a6
Showing 1 changed file with 9 additions and 1 deletion.
10 changes: 9 additions & 1 deletion src/node_file.cc
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,7 @@ FileHandle::TransferData::TransferData(int fd) : fd_(fd) {}
FileHandle::TransferData::~TransferData() {
if (fd_ > 0) {
uv_fs_t close_req;
CHECK_NE(fd_, -1);
CHECK_EQ(0, uv_fs_close(nullptr, &close_req, fd_, nullptr));
uv_fs_req_cleanup(&close_req);
}
Expand All @@ -237,8 +238,9 @@ BaseObjectPtr<BaseObject> FileHandle::TransferData::Deserialize(
// JS during GC. If closing the fd fails at this point, a fatal exception
// will crash the process immediately.
inline void FileHandle::Close() {
if (closed_) return;
if (closed_ || closing_) return;
uv_fs_t req;
CHECK_NE(fd_, -1);
int ret = uv_fs_close(env()->event_loop(), &req, fd_, nullptr);
uv_fs_req_cleanup(&req);

Expand Down Expand Up @@ -384,6 +386,7 @@ MaybeLocal<Promise> FileHandle::ClosePromise() {
close->Resolve();
}
}};
CHECK_NE(fd_, -1);
int ret = req->Dispatch(uv_fs_close, fd_, AfterClose);
if (ret < 0) {
req->Reject(UVException(isolate, ret, "close"));
Expand Down Expand Up @@ -555,8 +558,13 @@ ShutdownWrap* FileHandle::CreateShutdownWrap(Local<Object> object) {
}

int FileHandle::DoShutdown(ShutdownWrap* req_wrap) {
if (closing_ || closed_) {
req_wrap->Done(0);
return 1;
}
FileHandleCloseWrap* wrap = static_cast<FileHandleCloseWrap*>(req_wrap);
closing_ = true;
CHECK_NE(fd_, -1);
wrap->Dispatch(uv_fs_close, fd_, uv_fs_callback_t{[](uv_fs_t* req) {
FileHandleCloseWrap* wrap = static_cast<FileHandleCloseWrap*>(
FileHandleCloseWrap::from_req(req));
Expand Down

0 comments on commit a04f9a6

Please sign in to comment.