Closing filehandle after being used in a stream exits node without error message

[mshopf]

Version

20.3.0

Platform

Linux zuse1.efi.fh-nuernberg.de 5.4.17-2136.319.1.4.el8uek.x86_64 #2 SMP Mon Jun 5 13:39:13 PDT 2023 x86_64 x86_64 x86_64 GNU/Linux

Subsystem

streams

What steps will reproduce the bug?

I use a stream for writing into a filehandle (of a regular file). After closing the stream (not the filehandle!) I write to other positions in the file.
When I now close the file, node simply exists with return code 0. Following code is NOT executed.

const fs_p     = require('node:fs').promises;
const fs       = require('node:fs');
const stream_p = require('node:stream').promises;

async function test() {
    // Create writeable filehandle
    var fh = await fs_p.open ('/tmp/ttt.txt', 'w', 0o644);

    // Create stream, use stream, end stream, wait for finish
    var st = fh.createWriteStream ({autoClose: false});
    st.on ('error', (e) => {throw e});
    // No need to test return value with this little data
    st.write ('this is a filehandle / stream test');
    st.end ();
    await stream_p.finished (st);

    // Continue using filehandle, write something a beginning of file
    await fh.write ("TEST", 0);

    // Close filehandle to flush data and reuse system ressources
    console.log ('pre-close');

    // It is supposed to work like that.
    // filehandle.close() returns a Promise that is fullfilled with undefined after closing.
    // However, node simply exits (return code 0). Following code is not executed
    await fh.close ();

    // Working alternative. But only fixes system ressources, not node ressources.
    // And it blocks.
    //   fs.closeSync (fh.fd);

    // Continue
    console.log ('post-close');
    // should throw error because fd is closed
    await fh.write ("FH", 5);
}
test();

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

Reproduces always. Tried on v16, v18, v20.

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

Output:

pre-close
post-close
node:internal/process/promises:[...]
[Error: EBADF: bad file descriptor, write] { [...]

Content of /tmp/ttt.txt:

TEST is a filehandle / stream test

Works correctly with fs.closeSync (fh.fd); enabled instead of await fh.close();
Also works as expected, if the whole stream related block is commented out (contents of ttt.txt will differ).

What do you see instead?

Output:

pre-close

Content of /tmp/ttt.txt:

TEST is a filehandle / stream test

Additional information

No response

[aduh95]

The issue is we "reffing" the file handle here:

node/lib/internal/fs/streams.js

Line 139 in 9f19253

stream[kHandle][kRef]();

And we are never "unreffing" it until stream.close is called:

node/lib/internal/fs/streams.js

Line 89 in 9f19253

handle[kUnref]();

In your case, I think you expect stream.end() to unref file handle. If we can do that in a non-breaking way, that'd be great I think. PR's welcome :)

[mshopf]

Sounds reasonable. Unfortunately, I'm not fluent in the node base classes codebase...
And the potential side effects of such a change.
If I'd do a patch, it would probably be a breaking one ;-)

[mshopf]

As far as I read the code, end() basically finishes a stream. So should the unref() happen when we receive the "finish" event? Probably not before, because data could still to be written...

[colemickens]

Is there a workaround for this? I'm trying to do a similar pattern here so that I can ensure filehandle.sync() is called before close(), but as seen here, node20 is just exiting 0 silently after the attempt to close.

With this added in node21 (#49886), I see a path for safe fs stream IO, but I don't really understand how reliable FS stream writes are possible in node20 with this unresolved bug.