Rework miral::launch_app_env() to avoid calling functions that are not async-signal-safe after fork()
#3591
Conversation
…not async-signal-safe after fork()
| mir::log_debug("Restoring sigmask"); | ||
| sigset_t all_signals; | ||
| sigfillset(&all_signals); | ||
| pthread_sigmask(SIG_UNBLOCK, &all_signals, nullptr); | ||
|
|
||
| execvp(exec_args[0], const_cast<char*const*>(exec_args.data())); | ||
| execvpe(exec_args[0], const_cast<char*const*>(exec_args.data()), const_cast<char*const*>(exec_env.data())); |
There was a problem hiding this comment.
execvpe,sadly, isn't async-signal-safe (presumably because the obvious implementation requires memory allocation?).
I believe the correct approach here is to do $PATH handling ourselves; pre-construct all the possible paths that would be searched through, and then in the child calling execve on each in turn, annoyingly needing to keep track of error returns.
What a faff!
There was a problem hiding this comment.
For added bonus giggles, if $PATH is unset, we should be looking in _CS_PATH. Dear lord!
There was a problem hiding this comment.
(For the record, none of the exec*p* functions are async-signal-safe)
There was a problem hiding this comment.
What a faff!
Indeed. This is something that library implementers could (and should) get right for the rest of us
There was a problem hiding this comment.
I don't think there's a need for memory allocation in "the obvious implementation": remember C99 has VLAs. (See https://codebrowser.dev/glibc/glibc/posix/execvpe.c.html for an example implementation.)
So requiring execvpe() to be async-signal-safe is plausible (but outside our authority).
The problem is that when you call ¹: Notably, they may have taken any number of internal libc locks, which will never be released in the child because the while the state of everything is duplicated only one thread of execution is forked by |
Fixes: #3494