Repository navigation
Dropping vfork support regressed performance when posix_spawn not available #303
Description
Activity
I'm rather reluctant to reintroduce
vforksupport for a few reasons:- In practice, it is quite tricky to use correctly since you need to guarantee that the forked child does not touch any memory of the forking parent before
exec'ing. While in principlerunProcessis a fairly simple piece of code which can be easily audited, it calls various bits oflibcwhich are much harder to say anything about. - It is easy to end up with privilege escalation issues when
vforkis used in a multithreaded program which usesvforkto spawn a child with reduced privileges. - POSIX.1-2008 has formally removed any specification of its behavior, making it a bit hard to depend upon
Ultimately, I wouldn't necessarily be opposed to reintroducing support assuming that we explicitly disable its use when
setuidorsetgidare requested. This would be sufficient to side-step (1) and (2), which should make it fairly safe (assuming the semantics given to it by POSIX.1-2001).- In practice, it is quite tricky to use correctly since you need to guarantee that the forked child does not touch any memory of the forking parent before
I don't claim to have a great amount of expertise here, so I'm happy to trust your judgement. I know there are cases in which
posix_spawnisn't used so we have to fall back to[v]fork, but I don't know how common those are (at least once #303 resolves the issue withposix_spawn_file_actions_addchdir). If they are sufficiently rare, perhaps it is indeed fine to stay away fromvfork.The problem ultimately is that there is a large amount (complete?) overlap between the cases that
posix_spawncan't handle and the cases thatvforkcan't (safely) handle. Consequently, it's not clear to me that we really gain much beyond maintenance burden by reintroducingvforksupport.Reacted by Adam Gundry- added a commit that references this issue
on Sep 23, 2024 Unfortunately I just tested this again using
process-1.6.19.0, and it seems that code settingcwdstill ends up usingforkrather thanposix_spawn. I think I see why and have a patch incoming.Here's a test program that exhibits the issue:
import System.Process import System.IO mkProcess :: IO (Maybe Handle, Maybe Handle, Maybe Handle, ProcessHandle) mkProcess = createProcess CreateProcess { cmdspec = RawCommand "/bin/true" [] , cwd = Just "." , env = Just [] , std_in = CreatePipe , std_out = CreatePipe , std_err = CreatePipe , close_fds = False , create_group = False , delegate_ctlc = False , detach_console = False , create_new_console = False , new_session = False , child_group = Nothing , child_user = Nothing , use_process_jobs = False } main :: IO () main = do (createdInH, createdOutH, createdErrorH, pHandle) <- mkProcess pure ()
Running this under
stracegivesclone(child_stack=NULL, flags=CLONE_CHILD_CLEARTID|CLONE_CHILD_SETTID|SIGCHLD, child_tidptr=0x7febb63e1a10) = 740508whereas a fixed version gives
clone3({flags=CLONE_VM|CLONE_VFORK, exit_signal=SIGCHLD, stack=0x7d84a9c4e000, stack_size=0x9000}, 88) = 747494- added a commit that references this issue
on Oct 4, 2026
#279 replaced
vforkcalls withfork, butforkcan perform substantially worse when the process has a large memory footprint.Ideally we would use
posix_spawninstead, but (when setting the cwd) this requiresposix_spawn_file_actions_addchdir. There seems to be an inconsistency as theconfigurescript looks forposix_spawn_file_actions_addchdirbutdo_spawn_posixtests for the_npversion. Presumably that should be fixed, though it isn't clear whetherglibchas added support forposix_spawn_file_actions_addchdir(without_np) yet; perhaps we should test for both the portable and non-portable versions?Since there may be other cases where
posix_spawncannot be used, I wonder if we should retainvforksupport as well (preferringposix_spawnif possible, and falling back onforkifvforkis not available).cc @bgamari