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

[Thomas Stuefe]

On Mon, 8 Mar 2021 06:50:18 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>> Hi Thomas,
>> 
>> I appreciate the reworking to avoid vfork for the signal case, but have some concerns with the mechanism and I'm not sure a new mechanism is really needed. See comments below.
>> 
>> Thanks,
>> David
>
>> _Mailing list message from [David Holmes](mailto:david.holmes at oracle.com) on [hotspot-dev](mailto:hotspot-dev at openjdk.java.net):_
>> 
>> On 8/03/2021 4:12 pm, Thomas Stuefe wrote:
>> 
>> > On Mon, 8 Mar 2021 05:37:37 GMT, David Holmes <dholmes at openjdk.org> wrote:
>> > > > Thomas Stuefe has updated the pull request incrementally with one additional commit since the last revision:
>> > > > tidy up
>> > > 
>> > > 
>> > > src/hotspot/share/runtime/thread.hpp line 826:
>> > > > 824:   static void SpinRelease(volatile int * Lock);
>> > > > 825:
>> > > > 826: #ifdef POSIX
>> > > 
>> > > 
>> > > I hate seeing this in shared code. It really belongs in platform-dependent thread code - but osThread_posix doesn't exist. :(
>> > > But do we need this - can't the existing use_vfork_if_available parameter instead be renamed and interpreted as not_in_a_signal_handler?
>> > 
>> > 
>> > But would having this functionality not make sense? We always argue about what is and is not allowed during signal handling; here, we may get a mechanism to actually assert it and e.g. prevent using malloc() or logging or whatever VM functionality may creep in.
>> 
>> I agree having IsInSignalHandler may be generally useful, my issue was
>> with how you actually did it - in the shared code - and so simpler
>> solution for the currnet case would just be to reuse the existing para,eter.
>> 
>> > But I will revert my last commit completely and re-add the needs-vfork parameter. This was supposed to be mainly a cleanup; maybe it was wrong to mix in behavioral changes.
>> 
>> True. Do the refactoring then look at enhancements later. Sorry for
>> making this harder.
>> 
> 
> Oh no problem, that's what reviews are for. There will be occasion enough for behavioral change RFEs later.
> 
> Cheers, Thomas

New version; restored the vfork control boolean parameter; the only behavioral change to before is that on AIX we always use vfork since this is fine for AIX, and we have no overcommit there so its more urgent in case of OOMs.

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

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