EBADF thrown by stream if file is explicitly closed in v15

[alekitto]

An unexpected EBADF error is thrown if a stream is open on a file and then the file is explicitly closed.
The explicit close call is required if using fs/promises, otherwise a warning is emitted.

• Version: v15.0.1
• Platform: Darwin Kernel Version 19.6.0: Mon Aug 31 22:12:52 PDT 2020; root:xnu-6153.141.2~1/RELEASE_X86_64 x86_64 (macOS Catalina 10.15.7)
• Subsystem: fs

What steps will reproduce the bug?

I've created a simple case that generates the error.
TESTFILE.txt exists and could be empty.

const fs = require('fs');
const fsPromises = require('fs/promises');

(async () => {
    const handle = await fsPromises.open('TESTFILE.txt', 'r');
    const stream = fs.createReadStream(null, {
        fd: handle.fd,
        autoClose: false,
    });

    await handle.close();
})();

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

Always

What is the expected behavior?

No error.

What do you see instead?

The following error is thrown:

node:events:304
      throw er; // Unhandled 'error' event
      ^

Error: EBADF: bad file descriptor, read
Emitted 'error' event on ReadStream instance at:
    at emitErrorNT (node:internal/streams/destroy:194:8)
    at errorOrDestroy (node:internal/streams/destroy:257:7)
    at node:internal/fs/streams:193:9
    at FSReqCallback.wrapper [as oncomplete] (node:fs:539:5) {
  errno: -9,
  code: 'EBADF',
  syscall: 'read'
}

If autoClose is omitted or true, the thrown error is:

node:events:304
      throw er; // Unhandled 'error' event
      ^

Error: EBADF: bad file descriptor, close
Emitted 'error' event on ReadStream instance at:
    at emitErrorNT (node:internal/streams/destroy:194:8)
    at emitErrorCloseNT (node:internal/streams/destroy:159:3)
    at processTicksAndRejections (node:internal/process/task_queues:80:21) {
  errno: -9,
  code: 'EBADF',
  syscall: 'close'
}

This last error is thrown also if stream.destroy() is called before handle.close().

The only way to avoid the error is to not call handle.close, but this generates a deprecation warning in a more complex application (Closing a FileHandle object on garbage collection is deprecated.)

Additional information

No error is emitted on previous node versions (10-14).

[mmomtchev]

I think the right way to handle the close it is to wait for the end of the stream:

stream.on('end', () => handle.close());

I confirm that the behavior of Node 15 is different from the previous versions, but I think that the behavior in this situation is indeed undefined (@ronag ?), so it is only normal that it changes across different versions.

As a funny side note, when you use NODE_DEBUG=stream (it is probably timing dependent), the handle will get closed, than some housekeeping will immediately duplicate the stdin tty which will get assigned the same file handle id as the one closed and then reading will continue from stdin (and it will sometimes block the process exit) 😃
It appears to be a buggy behavior, but in fact it is simply undefined behavior as there is no mechanism that could notify a ReadStream that the simple number you passed as a file descriptor is no longer valid.

There is feature request for solving this: #35240

[ronag]

Yea, this is in the realm of undefined behavior. We could avoid the undefined behavior if the read stream took the FileHandle object instead of the fd directly.

But I think this is actually the correct behavior once it's no longer undefined.

[ronag]

See #35240

[mmomtchev]

On a second thought, reading from another file descriptor, even if the behavior is officially undefined, this is probably a little bit too much

• it is a silent error
• in a multi-context application it will corrupt not only the affected context, but also another, unrelated context
• if the other context is, let's say, a require() for lazy-loading a feature, the whole application will crash

There is a (quite complex) solution possible by reusing the trackUnamagedFds mechanism

[watson]

I think this can be closed right? When I run the supplied code on Node.js 15.0.1 I do get an error, but when I run it on the latest Node.js 15.x (or any newer version), the script exists without any errors. So I assume it's fixed.