RFR: JDK-8262955: Unify os::fork_and_exec() across Posix platforms

[Harold Seigel]

On Fri, 5 Mar 2021 07:24:34 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>> `os::fork_and_exec()` can be used from within the hotspot to start a child process. It is only called in fatal situations, in two cases:
>> a) to automatically start a debugger when ShowMessageBoxOnError is specified (uses *fork*())
>> b) to start a caller provided binary on OOM if -XX:OnOutOfMemoryError is specified (uses *vfork*())
>> 
>> The variants for AIX, Linux, Bsd are almost completely identical. So, this function can be unified under posix.
>> 
>> In addition to that, this patch does a number of small changes:
>> 
>> 1) Before, whether we would vfork() only on Linux and only for case (b). I changed this to always use vfork unconditionally, on all platforms, because: 
>>   - even though vfork() can be unsafe, the way we use it - calling  vfork()->exec()->_exit() with no intermediate steps - is safe.
>>   - Using vfork is good for OOM situations on all platforms, not just Linux, and also for starting the debugger in non-OOM cases. Keep in mind that we do this only for cases where the parent VM is about to die, so even if it were unsafe, the damage would be limited.
>> 2) I added a comment to the function to not use it outside of fatal error situations.
>> 3) I added a posix wrapper for getting the environ pointer, to hide MacOS specifics, and used it in two places to unify that coding.
>> 4) consistently used global scope :: for posix APIs.
>> 
>> Note that if we wanted to make os::fork_and_exec() a first class function, always safe to use, we should modify it to at least not leak any parent process file descriptors. Possibly safest would be to completely rewrite this function and use posix_spawn(). posix_spawn() we use in Runtime.exec() by default since JDK 13 (1). But as long as this is spawned by only dying VMs I think this function is fine.
>> 
>> ----
>> 
>> Tests: GAs, manual tests using -XX:ShowMessageBoxOnError
>
>> _Mailing list message from [David Holmes](mailto:david.holmes at oracle.com) on [hotspot-dev](mailto:hotspot-dev at openjdk.java.net):_
>> 
>> Hi Thomas,
>> 
>> On 5/03/2021 4:22 pm, Thomas Stuefe wrote:
>> 
>> > `os::fork_and_exec()` can be used from within the hotspot to start a child process. It is only called in fatal situations, in two cases:
>> > a) to automatically start a debugger when ShowMessageBoxOnError is specified (uses *fork*())
>> > b) to start a caller provided binary on OOM if -XX:OnOutOfMemoryError is specified (uses *vfork*())
>> > The variants for AIX, Linux, Bsd are almost completely identical. So, this function can be unified under posix.
>> > In addition to that, this patch does a number of small changes:
>> > 1) Before, whether we would vfork() only on Linux and only for case (b). I changed this to always use vfork unconditionally, on all platforms, because:
>> > - even though vfork() can be unsafe, the way we use it - calling  vfork()->exec()->_exit() with no intermediate steps - is safe.
>> 
>> Is it? The reason we use fork() for the error/crash case is because it
>> can get launched from a signal handling context and vfork is not
>> async-signal-safe.
>> 
>> There is some commentary in:
>> 
>> https://bugs.openjdk.java.net/browse/JDK-8027434
>> 
>> Cheers,
>> David
> 
> Hi David,
> 
> My estimate of vfork being safe here comes from experience. At SAP, at one time we replaced the jdk's Runtime.exec() implementation completely with our own; it lived in hotspot and was used by both the jdk and the hotspot, in and out of signal contexts. Our implementation mainly used vfork(), with a lot of safe guards of course (mainly a forkhelper binary, similarly to what Runtime.exec() does today).
> 
> I cannot completely exclude the possibility of problems here, but calling vfork()->exec()->_exit() is as safe as it gets. Seeing that we only use it for starting a debugger in case the original VM crashed, I think the possibility of problems is remote.
> 
> OTOH when starting a debugger spawned from a fat process, you may run into the same problems as with https://bugs.openjdk.java.net/browse/JDK-8027434. You don't want the machine to go into swap when trying to start the debugger for you.
> 
> I won't fight you on this though if you insist; mainly what I disliked was the introduction of Posix terminology in Windows code ("fork_and_exec" "use_vfork") and that can be straightened out by a separate layer (e,g. os::start_child_process(bool from_error_handler) -> os::Posix::os_fork_and_exec(can_use_vfork)).
> 
> Cheers, Thomas

Hi Thomas,
Can the #includes of <sys/wait.h> be removed from the os_aix.cpp, os_bsd.cpp, and os_linux.cpp files?
Thanks, Harold

-------------

PR: https://git.openjdk.java.net/jdk/pull/2810