From f6e22849269b8924658c1cd9f53ef8c4d0f04a5c Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Thu, 7 Mar 2024 17:28:25 +0100 Subject: [PATCH 1/5] fs: defer the writestream 'open' event by a nextTick Fixes: https://github.com/nodejs/node/issues/51993 --- lib/internal/fs/streams.js | 10 +++++-- .../test-fs-writestream-open-write.js | 27 +++++++++++++++++++ 2 files changed, 35 insertions(+), 2 deletions(-) create mode 100644 test/parallel/test-fs-writestream-open-write.js diff --git a/lib/internal/fs/streams.js b/lib/internal/fs/streams.js index 43f06d0104de61..163d0ab11d805b 100644 --- a/lib/internal/fs/streams.js +++ b/lib/internal/fs/streams.js @@ -75,13 +75,19 @@ function _construct(callback) { } else { stream.fd = fd; callback(); - stream.emit('open', stream.fd); - stream.emit('ready'); + // Ready must be deferred to the next tick, because callback will schedule + // some microtick work that we need to complete first. + process.nextTick(emitReady, stream); } }); } } +function emitReady(stream) { + stream.emit('open', stream.fd); + stream.emit('ready'); +} + // This generates an fs operations structure for a FileHandle const FileHandleOperations = (handle) => { return { diff --git a/test/parallel/test-fs-writestream-open-write.js b/test/parallel/test-fs-writestream-open-write.js new file mode 100644 index 00000000000000..d0833247ba5832 --- /dev/null +++ b/test/parallel/test-fs-writestream-open-write.js @@ -0,0 +1,27 @@ +'use strict' + +const common = require('../common'); +const { strictEqual } = require('assert'); +const { tmpdir } = require('os'); +const { join } = require('path'); +const fs = require('fs'); + +// Regression test for https://github.com/nodejs/node/issues/51993 + +const file = join(tmpdir(), `test-fs-writestream-open-write-${process.pid}.txt`); + +const w = fs.createWriteStream(file); + +w.on('open', common.mustCall(() => { + w.write('hello'); + + process.nextTick(() => { + w.write('world'); + w.end(); + }); +})); + +w.on('close', common.mustCall(() => { + strictEqual(fs.readFileSync(file, 'utf8'), 'helloworld'); + fs.unlinkSync(file); +})); From 97beaa026cf13f5752d453ffbe6c71040989867e Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Thu, 7 Mar 2024 18:17:34 +0100 Subject: [PATCH 2/5] fixup Signed-off-by: Matteo Collina --- lib/internal/fs/streams.js | 10 ++-------- lib/internal/streams/destroy.js | 2 +- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/lib/internal/fs/streams.js b/lib/internal/fs/streams.js index 163d0ab11d805b..43f06d0104de61 100644 --- a/lib/internal/fs/streams.js +++ b/lib/internal/fs/streams.js @@ -75,19 +75,13 @@ function _construct(callback) { } else { stream.fd = fd; callback(); - // Ready must be deferred to the next tick, because callback will schedule - // some microtick work that we need to complete first. - process.nextTick(emitReady, stream); + stream.emit('open', stream.fd); + stream.emit('ready'); } }); } } -function emitReady(stream) { - stream.emit('open', stream.fd); - stream.emit('ready'); -} - // This generates an fs operations structure for a FileHandle const FileHandleOperations = (handle) => { return { diff --git a/lib/internal/streams/destroy.js b/lib/internal/streams/destroy.js index 28802cae5eff32..23783d54b9b522 100644 --- a/lib/internal/streams/destroy.js +++ b/lib/internal/streams/destroy.js @@ -291,7 +291,7 @@ function constructNT(stream) { } else if (err) { errorOrDestroy(stream, err, true); } else { - process.nextTick(emitConstructNT, stream); + emitConstructNT(stream); } } From f94cf999bcea332209a1173c93a278adb56283aa Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Thu, 7 Mar 2024 18:21:18 +0100 Subject: [PATCH 3/5] fixup Signed-off-by: Matteo Collina --- lib/internal/streams/destroy.js | 6 +----- test/parallel/test-fs-writestream-open-write.js | 2 +- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/lib/internal/streams/destroy.js b/lib/internal/streams/destroy.js index 23783d54b9b522..96e61491f08cfc 100644 --- a/lib/internal/streams/destroy.js +++ b/lib/internal/streams/destroy.js @@ -291,7 +291,7 @@ function constructNT(stream) { } else if (err) { errorOrDestroy(stream, err, true); } else { - emitConstructNT(stream); + stream.emit(kConstruct); } } @@ -304,10 +304,6 @@ function constructNT(stream) { } } -function emitConstructNT(stream) { - stream.emit(kConstruct); -} - function isRequest(stream) { return stream?.setHeader && typeof stream.abort === 'function'; } diff --git a/test/parallel/test-fs-writestream-open-write.js b/test/parallel/test-fs-writestream-open-write.js index d0833247ba5832..770517f780ad01 100644 --- a/test/parallel/test-fs-writestream-open-write.js +++ b/test/parallel/test-fs-writestream-open-write.js @@ -1,4 +1,4 @@ -'use strict' +'use strict'; const common = require('../common'); const { strictEqual } = require('assert'); From 50e0c3c77aac31cd0d8bca03cd0ad36126bba92b Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Fri, 8 Mar 2024 09:56:15 +0100 Subject: [PATCH 4/5] fixup Signed-off-by: Matteo Collina --- test/parallel/test-fs-writestream-open-write.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test/parallel/test-fs-writestream-open-write.js b/test/parallel/test-fs-writestream-open-write.js index 770517f780ad01..ddacb5c618bd89 100644 --- a/test/parallel/test-fs-writestream-open-write.js +++ b/test/parallel/test-fs-writestream-open-write.js @@ -1,14 +1,16 @@ 'use strict'; const common = require('../common'); +const tmpdir = require('../common/tmpdir'); const { strictEqual } = require('assert'); -const { tmpdir } = require('os'); const { join } = require('path'); const fs = require('fs'); // Regression test for https://github.com/nodejs/node/issues/51993 -const file = join(tmpdir(), `test-fs-writestream-open-write-${process.pid}.txt`); +tmpdir.refresh(); + +const file = join(tmpdir.path, `test-fs-writestream-open-write.txt`); const w = fs.createWriteStream(file); From 3a44346f4c61190dcd5986e38ab124a15df60042 Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Fri, 8 Mar 2024 12:36:58 +0100 Subject: [PATCH 5/5] fixup Signed-off-by: Matteo Collina --- test/parallel/test-fs-writestream-open-write.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/parallel/test-fs-writestream-open-write.js b/test/parallel/test-fs-writestream-open-write.js index ddacb5c618bd89..af02d90ae6efd9 100644 --- a/test/parallel/test-fs-writestream-open-write.js +++ b/test/parallel/test-fs-writestream-open-write.js @@ -3,14 +3,13 @@ const common = require('../common'); const tmpdir = require('../common/tmpdir'); const { strictEqual } = require('assert'); -const { join } = require('path'); const fs = require('fs'); // Regression test for https://github.com/nodejs/node/issues/51993 tmpdir.refresh(); -const file = join(tmpdir.path, `test-fs-writestream-open-write.txt`); +const file = tmpdir.resolve('test-fs-writestream-open-write.txt'); const w = fs.createWriteStream(file);