Skip to content

Commit 59c7463

Browse files
ishaanlabs-ggIshaan
authored andcommitted
fs: remove FileHandle stream close listener
1 parent 75aec8e commit 59c7463

2 files changed

Lines changed: 107 additions & 1 deletion

File tree

lib/internal/fs/streams.js

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,8 @@ const kIsPerformingIO = Symbol('kIsPerformingIO');
4545

4646
const kFs = Symbol('kFs');
4747
const kHandle = Symbol('kHandle');
48+
const kHandleCloseListener = Symbol('kHandleCloseListener');
49+
const kHandleCloseListenerCleanup = Symbol('kHandleCloseListenerCleanup');
4850

4951
function _construct(callback) {
5052
const stream = this;
@@ -152,14 +154,38 @@ function importFd(stream, options) {
152154
stream[kHandle] = options.fd;
153155
stream[kFs] = FileHandleOperations(stream[kHandle]);
154156
stream[kHandle][kRef]();
155-
options.fd.on('close', FunctionPrototypeBind(stream.close, stream));
157+
if (options.autoClose === false) {
158+
stream[kHandleCloseListener] = FunctionPrototypeBind(stream.close, stream);
159+
stream[kHandleCloseListenerCleanup] = FunctionPrototypeBind(
160+
cleanupHandleCloseListener, stream);
161+
options.fd.once('close', stream[kHandleCloseListener]);
162+
stream.once('close', stream[kHandleCloseListenerCleanup]);
163+
stream.once('end', stream[kHandleCloseListenerCleanup]);
164+
stream.once('finish', stream[kHandleCloseListenerCleanup]);
165+
stream.once('error', stream[kHandleCloseListenerCleanup]);
166+
}
156167
return options.fd.fd;
157168
}
158169

159170
throw new ERR_INVALID_ARG_TYPE('options.fd',
160171
['number', 'FileHandle'], options.fd);
161172
}
162173

174+
function cleanupHandleCloseListener() {
175+
if (this[kHandleCloseListener] === undefined) {
176+
return;
177+
}
178+
179+
this[kHandle].removeListener('close', this[kHandleCloseListener]);
180+
this[kHandle][kUnref]();
181+
this.removeListener('close', this[kHandleCloseListenerCleanup]);
182+
this.removeListener('end', this[kHandleCloseListenerCleanup]);
183+
this.removeListener('finish', this[kHandleCloseListenerCleanup]);
184+
this.removeListener('error', this[kHandleCloseListenerCleanup]);
185+
this[kHandleCloseListener] = undefined;
186+
this[kHandleCloseListenerCleanup] = undefined;
187+
}
188+
163189
function ReadStream(path, options) {
164190
if (!(this instanceof ReadStream))
165191
return new ReadStream(path, options);

test/parallel/test-fs-promises-file-handle-stream.js

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,87 @@ async function validateRead() {
4242
);
4343
}
4444

45+
async function validateReadStreamReleasesFileHandleCloseListener() {
46+
const filePathForHandle = path.resolve(tmpDir, 'tmp-read-listener.txt');
47+
const buf = Buffer.from('Hello world', 'utf8');
48+
49+
fs.writeFileSync(filePathForHandle, buf);
50+
51+
const fileHandle = await open(filePathForHandle);
52+
53+
for (let i = 0; i < buf.length; i++) {
54+
await buffer(fileHandle.createReadStream({
55+
start: i,
56+
end: i,
57+
autoClose: false,
58+
}));
59+
60+
assert.strictEqual(fileHandle.listenerCount('close'), 0);
61+
}
62+
63+
await fileHandle.close();
64+
}
65+
66+
async function validateWriteStreamReleasesFileHandleCloseListener() {
67+
const filePathForHandle = path.resolve(tmpDir, 'tmp-write-listener.txt');
68+
const buf = Buffer.from('Hello world', 'utf8');
69+
70+
const fileHandle = await open(filePathForHandle, 'w');
71+
72+
for (let i = 0; i < buf.length; i++) {
73+
const stream = fileHandle.createWriteStream({
74+
start: i,
75+
autoClose: false,
76+
});
77+
stream.end(buf.subarray(i, i + 1));
78+
await finished(stream);
79+
80+
assert.strictEqual(fileHandle.listenerCount('close'), 0);
81+
}
82+
83+
await fileHandle.close();
84+
assert.deepStrictEqual(fs.readFileSync(filePathForHandle), buf);
85+
}
86+
87+
async function validateReadStreamAutoCloseClosesFileHandle() {
88+
const filePathForHandle = path.resolve(tmpDir, 'tmp-read-auto-close.txt');
89+
const buf = Buffer.from('Hello world', 'utf8');
90+
91+
fs.writeFileSync(filePathForHandle, buf);
92+
93+
const fileHandle = await open(filePathForHandle);
94+
const closed = new Promise((resolve) => {
95+
fileHandle.once('close', common.mustCall(resolve));
96+
});
97+
98+
assert.deepStrictEqual(await buffer(fileHandle.createReadStream()), buf);
99+
await closed;
100+
assert.strictEqual(fileHandle.listenerCount('close'), 0);
101+
}
102+
103+
async function validateWriteStreamAutoCloseClosesFileHandle() {
104+
const filePathForHandle = path.resolve(tmpDir, 'tmp-write-auto-close.txt');
105+
const buf = Buffer.from('Hello world', 'utf8');
106+
107+
const fileHandle = await open(filePathForHandle, 'w');
108+
const closed = new Promise((resolve) => {
109+
fileHandle.once('close', common.mustCall(resolve));
110+
});
111+
const stream = fileHandle.createWriteStream();
112+
113+
stream.end(buf);
114+
await finished(stream);
115+
await closed;
116+
117+
assert.strictEqual(fileHandle.listenerCount('close'), 0);
118+
assert.deepStrictEqual(fs.readFileSync(filePathForHandle), buf);
119+
}
120+
45121
Promise.all([
46122
validateWrite(),
47123
validateRead(),
124+
validateReadStreamReleasesFileHandleCloseListener(),
125+
validateWriteStreamReleasesFileHandleCloseListener(),
126+
validateReadStreamAutoCloseClosesFileHandle(),
127+
validateWriteStreamAutoCloseClosesFileHandle(),
48128
]).then(common.mustCall());

0 commit comments

Comments
 (0)