Repository navigation
Conversation
| continue; | ||
|
|
||
| if (::close(conv) != 0) | ||
| while (::close(conv) == -1 && errno != EBADF) |
There was a problem hiding this comment.
::close(conv) != 0 maybe ? It seems more portable and preserves the original code.
The same remark applies to V2, but this one is specific to openbsd atm.
Any opinion ?
There was a problem hiding this comment.
I think just ignoring BADF is the right call, I don't like the while loop there, just for readability. Maybe we should do it like this?
if (::close(conv) != 0 && errno != EBADF)
{
while (errno == EINTR && close(conv) == -1);
}There was a problem hiding this comment.
I just notice the change I made isn't correct in cases of EINTR.
Also, the loop call close() instead of ::close().
Actually, you mean we should drop exec.set_error() ?
Then we don't need if { while() } construct, just this ?
while (::close(conv) == -1 && errno == EINTR);
OpenBSD's /dev/fd contains static device nodes rather than a list of open descriptors. Enumerating it can encounter closed descriptors and miss open descriptors beyond the available nodes. In Process v2, use close() between retained descriptors and closefrom() above the last one, avoiding /dev/fd enumeration. For Process v1, deliberately keep the fix minimal: ignore EBADF and retry close() on EINTR, rather than rewriting the cleanup algorithm to follow the v2 approach. This prevents limit_handles failures but leaves descriptors missing from /dev/fd unhandled.
328dd25 to
4b37b73
Compare
| // gives us | ||
| // | ||
| // int closefrom(int fd); | ||
| // void closefrom(int lowfd); |
There was a problem hiding this comment.
Just notice that FreeBSD closefrom differ from OpenBSD; see the comment below
| { | ||
| errno = 0; | ||
| ::closefrom(whitelist.back() + 1); | ||
| } while (errno == EINTR); |
There was a problem hiding this comment.
This is trade-off to handle both FreeBSD and OpenBSD implementation.
Initially this was meant to be small; should we implement yet another close_all() ?
There was a problem hiding this comment.
That should be #if-defed, because resetting errno like that is very uncommon.
OpenBSD's /dev/fd contains static device nodes rather than a list of open descriptors. Enumerating it can encounter closed descriptors and miss open descriptors beyond the available nodes.
In Process v2, use close() between retained descriptors and closefrom() above the last one, avoiding /dev/fd enumeration.
For Process v1, deliberately keep the fix minimal: ignore EBADF and retry close() on EINTR, rather than rewriting the cleanup algorithm to follow the v2 approach. This prevents limit_handles failures but leaves descriptors missing from /dev/fd unhandled.