GeezerVison

Review requested:

• @nodejs/startup

by hoping the next file descriptor to be given will be the one which doesn't exist

“hoping” is doing a lot of work here 🙂 This was actually spec’d behavior in previous POSIX versions, and I was quite surprised to learn (through this PR) that the spec actually changed in this regard.

I’d leave a comment in the source code about this, since others might be surprised by this as well.

I’d leave a comment in the source code about this, since others might be surprised by this as well.

Got you. I'll do that in a little bit ✌️

CI: https://ci.nodejs.org/job/node-test-pull-request/46354/

The "open() returns the lowest available file descriptor" convention is so deeply codified in the UNIX ecosystem that it's never going away, no matter what POSIX says.

While I do agree with this, there still are specific cases where "available" means something different to what's expected. Case in point:

#include <fcntl.h>
#include <stdio.h>
#include <stdlib.h>

int main(void) {
	system("echo && fstat | grep a.out");
	int fd = open("/dev/null", O_RDWR);
	printf("\n/dev/null: %d\n", fd);

	return 0;
}

Running this on FreeBSD in a separate session with daemon -o log.log -r /tmp/a.out and then closing said session (thus making stdin invalid):

root     a.out      89214 text [restricted]    519 -rwxr-xr-x   15424  r
root     a.out      89214   wd /        107129 drwxr-xr-x      16  r
root     a.out      89214 root /        107106 drwxr-xr-x      24  r
root     a.out      89214 jail /        107106 drwxr-xr-x      24  r
root     a.out      89214    0 -         -         bad    -
root     a.out      89214    1* pipe fffff8002e0e7d00 <-> fffff8002e0e7ba0      0 rw
root     a.out      89214    2* pipe fffff8002e0e7d00 <-> fffff8002e0e7ba0      0 rw
root     a.out      89214    3 [restricted]     24 crw-rw-rw-    null rw

/dev/null: 3

(This is actually the issue nodejs/help#2411 had.)

Here, 0 should be available, yet /dev/null is still opened as 3. Something else that works is closing 0 before opening /dev/null, but IMHO at that point it's kinda relying on flaky behaviour.

Ah okay, fair enough. Fd 0 points to a kind of zombie file description in your example, i.e., it's technically still open but not actually usable?

Fd 0 points to a kind of zombie file description in your example, i.e., it's technically still open but not actually usable?

AFAIU the file descriptor is never actually closed, so it isn't ever marked as unused (fdunused()), which is why it's never removed from the file descriptor table.

Wait, but fstat() still fails with EBADF? How does that work?

I don't know, there are plenty of places along the line where fstat(2) can fail with EBADF, not just because a file descriptor is not found in the file descriptor table (you can check out kern_fstat(), my quick guess is that fp->f_ops is set to &badfileops at some point before being unused).

I could investigate further if you'd like an exact answer but I don't see how it's too relevant outside of the FreeBSD kernel :P

"Zombie file descriptor" is good enough for me. :-)

CI: https://ci.nodejs.org/job/node-test-pull-request/46479/

Is there anything left for me to do here?

CI: https://ci.nodejs.org/job/node-test-pull-request/47466/

Landed in 0c8b141