From 31322ee29baa26d73059593a49c8ea76e6d5fc22 Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Sat, 20 Jun 2020 00:12:44 +0200 Subject: [PATCH 1/2] fs: document why isPerformingIO is required --- lib/internal/fs/streams.js | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/lib/internal/fs/streams.js b/lib/internal/fs/streams.js index 9e6050139dc79a..f5072082c340b6 100644 --- a/lib/internal/fs/streams.js +++ b/lib/internal/fs/streams.js @@ -255,6 +255,12 @@ ReadStream.prototype._read = function(n) { }; ReadStream.prototype._destroy = function(err, cb) { + // Usually for async io it is safe to close a file descriptor + // even when there are pending operations. However, due to platform + // differences file IO is implemented using synchronous operations + // running in a thread pool. Therefore, file descriptors are not safe + // to close while used in a pending read or write operation. Wait for + // any pending IO (kIsPerformingIO) to complete (kIoDone). if (this[kIsPerformingIO]) { this.once(kIoDone, (er) => close(this, err || er, cb)); } else { @@ -416,12 +422,19 @@ WriteStream.prototype._writev = function(data, cb) { }; WriteStream.prototype._destroy = function(err, cb) { + // Usually for async io it is safe to close a file descriptor + // even when there are pending operations. However, due to platform + // differences file IO is implemented using synchronous operations + // running in a thread pool. Therefore, file descriptors are not safe + // to close while used in a pending read or write operation. Wait for + // any pending IO (kIsPerformingIO) to complete (kIoDone). if (this[kIsPerformingIO]) { this.once(kIoDone, (er) => close(this, err || er, cb)); } else { close(this, err, cb); } }; + WriteStream.prototype.close = function(cb) { if (cb) { if (this.closed) { From a444d349044eb97c292c39b4f90e07ccb2a66bd4 Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Sat, 20 Jun 2020 08:58:03 +0200 Subject: [PATCH 2/2] Apply suggestions from code review Co-authored-by: Trivikram Kamat <16024985+trivikr@users.noreply.github.com> --- lib/internal/fs/streams.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/internal/fs/streams.js b/lib/internal/fs/streams.js index f5072082c340b6..0209f3e844c759 100644 --- a/lib/internal/fs/streams.js +++ b/lib/internal/fs/streams.js @@ -255,7 +255,7 @@ ReadStream.prototype._read = function(n) { }; ReadStream.prototype._destroy = function(err, cb) { - // Usually for async io it is safe to close a file descriptor + // Usually for async IO it is safe to close a file descriptor // even when there are pending operations. However, due to platform // differences file IO is implemented using synchronous operations // running in a thread pool. Therefore, file descriptors are not safe @@ -422,7 +422,7 @@ WriteStream.prototype._writev = function(data, cb) { }; WriteStream.prototype._destroy = function(err, cb) { - // Usually for async io it is safe to close a file descriptor + // Usually for async IO it is safe to close a file descriptor // even when there are pending operations. However, due to platform // differences file IO is implemented using synchronous operations // running in a thread pool. Therefore, file descriptors are not safe