writableStream.destroy() closes the underlying file even when stream is created with {autoClose: false, emitClose: false}

Version

v18.17.1, v20.5.1

Platform

Linux 6.4.10-200.fc38.x86_64 #1 SMP PREEMPT_DYNAMIC Fri Aug 11 12:20:29 UTC 2023 x86_64 GNU/Linux

Subsystem

fs, stream

What steps will reproduce the bug?

Minimal program to reproduce.

Create 2 writeStreams from a FileHandle with {autoClose: false, emitClose: false} options. Write some data to the first stream and destroy it. Try to write some data to the second stream.

import { open } from 'node:fs/promises'
import { pipeline } from 'node:stream/promises'

async function main() {
    const destFile = await open('test.tmp', 'w')
    destFile.truncate(1024)
    const stream0 = destFile.createWriteStream({start: 0, autoClose: false, emitClose: false})
    const stream1 = destFile.createWriteStream({start: 512, autoClose: false, emitClose: false})
    stream0.on('error', (err) => { console.log('stream0 error', err) })
    await pipeline("Hello world from stream0", stream0)
    stream0.destroy(new Error('destroyed'))
    try {
        await pipeline("Hello world from stream1", stream1) // emits ERR_STREAM_WRITE_AFTER_END
    } finally {
        // destFile.fd is -1 here
        destFile.close()
    }
}

await main()

How often does it reproduce? Is there a required condition?

No response

What is the expected behavior? Why is that the expected behavior?

Data is written, FileHandle is not closed up until close() call because emitClose: true is specified on stream creation.

What do you see instead?

ERR_STREAM_WRITE_AFTER_END is emmited when trying to write to the steream1. destFile.fd is set to -1.

Additional information

No response

Possible duplicate of #45721? Can you check with strace -y if the file descriptor is actually closed?

You're destroying the stream this is expected behavior, it's not closed on its own you're explicitly destroying the stream...

@benjamingr

@benjamingr According to the docs:

By default, the stream will emit a 'close' event after it has been destroyed. Set the emitClose option to false to change this behavior.

@notorca

@notorca right, but the issue you're seeing isn't the "close" event being emitted it's that you're trying to reuse the file handle after destroying the streama

In fact for compatibility reasons for most of Node's history emitClose was set to false on fs streams, it was changed by ronag here: #31408

@benjamingr

@benjamingr It's still not clear for me why destroying a stream closes the underlying FileHandle. I suppose it should be controlled by emitClose, FileHandle and streams related as 1:N.

@rluvaton

As soon as you create a write stream from a file handle, you're tying that file to the stream and if you close one the other is closed - so the relationship is not in fact 1:N.

You misunderstand emitClose it has nothing to do with actually closing stuff - it's only about whether or not the close event should be emitted.

We have discussed having a "unref this stream so destroying it is a no-op" a bunch for various use cases (the one Ben linked to and a few others, see context in #48007 cc @rluvaton ) but we haven't been come up with an implementation that doesn't leak.

@benjamingr

@benjamingr As I can see emitClose is actually related to closing stuff. Here is a sample code showing that file handles and streams are actually 1:N (it's the same as in the original bug posted, but with close instead of destroy):

import { open } from 'node:fs/promises'
import { pipeline } from 'node:stream/promises'

async function main() {
    const destFile = await open('test1.tmp', 'w')
    destFile.truncate(1024)
    const stream0 = destFile.createWriteStream({start: 0, autoClose: false, emitClose: false})
    const stream1 = destFile.createWriteStream({start: 512, autoClose: false, emitClose: false})
    stream0.on('error', (err) => { console.log('stream0 error', err) })
    await pipeline("Hello world from stream0", stream0)
    stream0.close()
    try {
        await pipeline("Hello world from stream1", stream1)
    } finally {
        destFile.close()
    }
}

await main()

If "Hello world from stream1" exists in the file that's probably a bug we should fix (assuming it's not timing)

@bnoordhuis

@bnoordhuis Seems related to #45721. According to strace file descriptor is leaked in this case. So destroying the stream just sets fd to -1 in FileHandle without closing it.

@benjamingr

@benjamingr "Hello world from stream1" exists in the file. If you want to enforce 1:1 semantics (IMHO that's unreasonable restriction) it would be better to throw an error on attempt to create a second stream from the same FileHandle.

@ronag

@ronag am I wrong? Do we support 1:N fs.WriteStreams to FileHandles intentionally? If we do why does destroy act differently from end/close? (and why isn't .close on fs streams deprecated? Is there an actual reason to use it and not .end?)

I agree the current behavior with FileHandle is weird and I tried to fix it in #47484

IMO

.close should be deprecated.
FileHandle should expose ref/unref
fs streams should unref FileHandle not close it.

@rluvaton

All that sounds good to me, @rluvaton anything of those you want to work on?

I can work on this, doesn't really matter which :)

@rluvaton

@rluvaton all three, really picking up ronag's stale PR

I will try to get to this this weekend

so I'm trying to understand:

wouldn't unref will cause a leak as the file handle still exists or it will be GCed?
unref the file handles means that the stream is no longer affecting the file handle when destroyed but until that the stream stays open?

fs streams should unref FileHandle not close it.

should unref when destroyed?

the following test that was mention, when used destroy should write both strings to file?

import { open } from 'node:fs/promises'
import { pipeline } from 'node:stream/promises'

async function main() {
    const destFile = await open('test1.tmp', 'w')
    destFile.truncate(1024)
    const stream0 = destFile.createWriteStream({start: 0, autoClose: false, emitClose: false})
    const stream1 = destFile.createWriteStream({start: 512, autoClose: false, emitClose: false})
    stream0.on('error', (err) => { console.log('stream0 error', err) })
    await pipeline("Hello world from stream0", stream0)
    stream0.close()
    try {
        await pipeline("Hello world from stream1", stream1)
    } finally {
        destFile.close()
    }
}

await main()

Sorry I don't understand any of that.

Basically, the stream should ref the FileHandle and then unref it, if ref count === 0 (which would happen if no one except the stream has it) then the file handle will destroy the file descriptor.

Not sure where the whole GC thing comes in. Shouldn't be relevant.

Has there been any progress on this? I just encountered the issue on v21.7.1 except with readable streams. I create the streams with

filehandle.createReadStream({ autoClose: false, emitClose: false };

but calling destroy() on any of them still closes the underlying filehandle (or at least modifies it so that the fd becomes -1) despite the explicit options. Notably, no 'close' event is emitted from the readable stream but there is a 'close' event emitted from the filehandle.
Abridged example code from when I encountered it:

var read_stream = filehandle.createReadStream( { autoClose: false, emitClose: false, start: 0 } );
read_stream.on( 'close', () => {
	console.error( 'stream on close.' ); // Never run.
} );
filehandle.on( 'close', () => {
	console.error( 'filehandle on close.' ); // Run.
} );
console.log( 'Before destroying read stream: filehandle: %o', filehandle ); // filehandle.fd: 24
read_stream.destroy( null, () => { // I even tried fiddling with the undocumented callback in destroy() but it made no difference.
	console.log( 'In stream.destroy callback.' ); // Never run.
} );
console.log( 'After destroying read stream: filehandle: %o', filehandle ); // filehandle.fd: -1

Is there currently a way to create a readable stream from a filehandle that doesn't make the filehandle's lifespan terminally dependent on the stream?

I just ran into what looks like the same issue. But I call

createWriteStream("", { fd: fdTarget, autoClose: false }).

If I later do:

writeStream.destroy();
closeSync(fdTarget);

I get an EBADF error. An indication that the file descriptor was closed by destroy().

This issue has been marked as stale due to 90 days of inactivity.
It will be automatically closed in 30 days if no further activity occurs. If this is still relevant, please leave a comment or update it to keep it open.

This issue has been automatically closed after 30 days of inactivity following its stale status (no activity for a total of 120 days).
If this is still relevant, feel free to reopen it or leave a comment with additional details so we can continue the discussion.

Navigation Menu

Repository navigation

writableStream.destroy() closes the underlying file even when stream is created with {autoClose: false, emitClose: false} #49241

Version

Platform

Subsystem

What steps will reproduce the bug?

How often does it reproduce? Is there a required condition?

What is the expected behavior? Why is that the expected behavior?

What do you see instead?

Additional information

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

writableStream.destroy() closes the underlying file even when stream is created with {autoClose: false, emitClose: false} #49241

Description

Version

Platform

Subsystem

What steps will reproduce the bug?

How often does it reproduce? Is there a required condition?

What is the expected behavior? Why is that the expected behavior?

What do you see instead?

Additional information

Activity

bnoordhuis commented on Aug 19, 2023

benjamingr commented on Aug 19, 2023

notorca commented on Aug 19, 2023

benjamingr commented on Aug 19, 2023

benjamingr commented on Aug 19, 2023

notorca commented on Aug 19, 2023

benjamingr commented on Aug 19, 2023

notorca commented on Aug 19, 2023

benjamingr commented on Aug 19, 2023

notorca commented on Aug 19, 2023

notorca commented on Aug 19, 2023

benjamingr commented on Aug 20, 2023

ronag commented on Aug 21, 2023

benjamingr commented on Aug 21, 2023

rluvaton commented on Aug 21, 2023

benjamingr commented on Aug 21, 2023

rluvaton commented on Aug 22, 2023

rluvaton commented on Aug 25, 2023

ronag commented on Aug 25, 2023

Anadian commented on Jun 4, 2024

erikjalevik commented on Apr 12, 2026

github-actions commented on Jul 20, 2026

github-actions commented on Aug 20, 2026

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions