diff --git a/lib/internal/fs/promises.js b/lib/internal/fs/promises.js index 41c17de95ed..6f5d2921a59 100644 --- a/lib/internal/fs/promises.js +++ b/lib/internal/fs/promises.js @@ -100,6 +100,7 @@ const kRefs = Symbol('kRefs'); const kClosePromise = Symbol('kClosePromise'); const kCloseResolve = Symbol('kCloseResolve'); const kCloseReject = Symbol('kCloseReject'); +const kClosing = Symbol('kClosing'); const kRef = Symbol('kRef'); const kUnref = Symbol('kUnref'); const kLocked = Symbol('kLocked'); @@ -137,7 +138,17 @@ class FileHandle extends EventEmitterMixin(JSTransferable) { this[kFd] = filehandle ? filehandle.fd : -1; this[kRefs] = 1; - this[kClosePromise] = null; + this[kClosing] = false; + this[kClosePromise] = SafePromisePrototypeFinally( + new Promise((resolve, reject) => { + this[kCloseResolve] = resolve; + this[kCloseReject] = reject; + }), () => { + this[kClosePromise] = undefined; + this[kCloseReject] = undefined; + this[kCloseResolve] = undefined; + }, + ); } getAsyncId() { @@ -216,31 +227,21 @@ class FileHandle extends EventEmitterMixin(JSTransferable) { return PromiseResolve(); } - if (this[kClosePromise]) { + if (this[kClosing]) { return this[kClosePromise]; } - this[kRefs]--; - if (this[kRefs] === 0) { - this[kFd] = -1; - this[kClosePromise] = SafePromisePrototypeFinally( - this[kHandle].close(), - () => { this[kClosePromise] = undefined; }, - ); - } else { - this[kClosePromise] = SafePromisePrototypeFinally( - new Promise((resolve, reject) => { - this[kCloseResolve] = resolve; - this[kCloseReject] = reject; - }), () => { - this[kClosePromise] = undefined; - this[kCloseReject] = undefined; - this[kCloseResolve] = undefined; - }, - ); - } + this[kClosing] = true; + this[kUnref](); + + SafePromisePrototypeFinally( + this[kClosePromise], + () => { + this[kClosePromise] = undefined; + this.emit('close'); + } + ); - this.emit('close'); return this[kClosePromise]; }; @@ -252,7 +253,7 @@ class FileHandle extends EventEmitterMixin(JSTransferable) { readableWebStream() { if (this[kFd] === -1) throw new ERR_INVALID_STATE('The FileHandle is closed'); - if (this[kClosePromise]) + if (this[kClosing]) throw new ERR_INVALID_STATE('The FileHandle is closing'); if (this[kLocked]) throw new ERR_INVALID_STATE('The FileHandle is locked'); @@ -311,7 +312,7 @@ class FileHandle extends EventEmitterMixin(JSTransferable) { } [kTransfer]() { - if (this[kClosePromise] || this[kRefs] > 1) { + if (this[kClosing] || this[kRefs] > 1) { throw lazyDOMException('Cannot transfer FileHandle while in use', 'DataCloneError'); } @@ -337,11 +338,14 @@ class FileHandle extends EventEmitterMixin(JSTransferable) { } [kRef]() { + assert(this[kFd] !== -1); this[kRefs]++; } [kUnref]() { + assert(this[kFd] !== -1); this[kRefs]--; + if (this[kRefs] === 0) { this[kFd] = -1; PromisePrototypeThen( @@ -980,6 +984,7 @@ module.exports = { }, FileHandle, + kRefs, kRef, kUnref, }; diff --git a/lib/internal/fs/streams.js b/lib/internal/fs/streams.js index f75d0fba917..a5c300049fd 100644 --- a/lib/internal/fs/streams.js +++ b/lib/internal/fs/streams.js @@ -26,7 +26,7 @@ const { } = require('internal/validators'); const { errorOrDestroy } = require('internal/streams/destroy'); const fs = require('fs'); -const { kRef, kUnref, FileHandle } = require('internal/fs/promises'); +const { kRef, kUnref, kRefs, FileHandle } = require('internal/fs/promises'); const { Buffer } = require('buffer'); const { copyObject, @@ -36,6 +36,8 @@ const { } = require('internal/fs/utils'); const { Readable, Writable, finished } = require('stream'); const { toPathIfFileURL } = require('internal/url'); +const assert = require('internal/assert'); + const kIoDone = Symbol('kIoDone'); const kIsPerformingIO = Symbol('kIsPerformingIO'); @@ -80,15 +82,21 @@ function _construct(callback) { } // This generates an fs operations structure for a FileHandle -const FileHandleOperations = (handle) => { +const FileHandleOperations = (handle, closeHandler) => { return { open: (path, flags, mode, cb) => { throw new ERR_METHOD_NOT_IMPLEMENTED('open()'); }, close: (fd, cb) => { + handle.off('close', closeHandler); handle[kUnref](); - PromisePrototypeThen(handle.close(), - () => cb(), cb); + if (handle[kRefs] > 1) { + // Someone else has a ref to the handle. + process.nextTick(cb); + } else { + PromisePrototypeThen(handle.close(), + () => cb(), cb); + } }, read: (fd, buf, offset, length, pos, cb) => { PromisePrototypeThen(handle.read(buf, offset, length, pos), @@ -134,10 +142,12 @@ function importFd(stream, options) { // FileHandle is not supported with custom fs operations throw new ERR_METHOD_NOT_IMPLEMENTED('FileHandle with fs'); } + assert(options.fd[kRefs] > 0); + const closeHandler = FunctionPrototypeBind(stream.close, stream); stream[kHandle] = options.fd; - stream[kFs] = FileHandleOperations(stream[kHandle]); + stream[kFs] = FileHandleOperations(stream[kHandle], closeHandler); stream[kHandle][kRef](); - options.fd.on('close', FunctionPrototypeBind(stream.close, stream)); + stream[kHandle].on('close', closeHandler); return options.fd.fd; } diff --git a/test/parallel/test-fs-read-stream-file-handle.js b/test/parallel/test-fs-read-stream-file-handle.js index 810745149dc..2560b7e07f7 100644 --- a/test/parallel/test-fs-read-stream-file-handle.js +++ b/test/parallel/test-fs-read-stream-file-handle.js @@ -29,12 +29,22 @@ fs.promises.open(file, 'r').then((handle) => { fs.promises.open(file, 'r').then((handle) => { handle.on('close', common.mustCall()); const stream = fs.createReadStream(null, { fd: handle }); - stream.on('data', common.mustNotCall()); + stream.on('data', common.mustCall(() => { + stream.destroy(); + })); stream.on('close', common.mustCall()); return handle.close(); }).then(common.mustCall()); +fs.promises.open(file, 'r').then((handle) => { + handle.on('close', common.mustCall()); + const stream = fs.createReadStream(null, { fd: handle }); + stream.on('data', common.mustNotCall()); + stream.on('close', common.mustCall()); + stream.destroy(); +}).then(common.mustCall()); + fs.promises.open(file, 'r').then((handle) => { handle.on('close', common.mustCall()); const stream = fs.createReadStream(null, { fd: handle });