haskell / process

Library for dealing with system processes
http://hackage.haskell.org/package/process
Other
87 stars 82 forks source link

Fix process#251 #257

Closed bgamari closed 2 years ago

bgamari commented 2 years ago

Previously to spawn a process with a closed standard handle, we would use posix_spawn_file_action_addclose. However, it turns out that POSIX specifies that spawnp() may fail if addclose() is used on an fd that is already closed. While glibc and musl appear to ignore this aspect of the specification, Darwin indeed follows it leading to #251.

This behavior is rather unfortunate as posix_spawn_file_action_addclose is a convenient way to close a handle in a subprocess in a race-free manner (e.g. unlike O_CLOEXEC, which is global). To avoid #251 we must first use posix_spawn_file_action_addopen on the fd (e.g. opening /dev/null) to be closed to ensure that it is valid, which has the side-effect of closing the inherited fd. We can then safely use posix_spawn_file_action_addclose to close the fd.

Fixes #251.

bgamari commented 2 years ago

Unfortunately I found that the current Cabal-driven testsuite isn't particularly well-suited to test this, consequently I ended up adding a test in GHC's testsuite. Testing this locally as we speak.

bgamari commented 2 years ago

I have confirmed that the test reproduces #251 on Darwin without this change and that the change fixes it.

bgamari commented 2 years ago

Note that I ultimately opted not to make the addopen/addclose behavior conditional on platform since technically spawnp failing in this case is what is mandated by POSIX.

hasufell commented 2 years ago

Is this released? I can't find it in https://hackage.haskell.org/package/process-1.6.15.0/changelog