RFR 8027434: "-XX:OnOutOfMemoryError" uses fork instead of vfork

[David Holmes]

Hi Thomas,

On 27/09/2018 9:35 AM, Thomas Stüfe wrote:
> Hi,
> 
> I think there is no reason at all to not always to use vfork() here
> (in the linux implementation for fork_and_exec()).
> 
> The concerns with vfork() I outlined in
> http://mail.openjdk.java.net/pipermail/core-libs-dev/2018-September/055333.html
> do not apply here. This is a very simple case, no stdio redirection,
> no environment change, nothing. So, I would just exchange fork() for
> vfork().

What about signal-safety if called from the signal handler in the 
onError case?

Thanks,
David

> Best Regards, Thomas
> 
> On Thu, Sep 27, 2018 at 3:22 PM, David Holmes <david.holmes at oracle.com> wrote:
>> Hi,
>>
>> On 27/09/2018 8:18 AM, Muthusamy Chinnathambi wrote:
>>>
>>> Hi David,
>>>
>>>> That doesn't work.
>>>
>>> I get your point. My bad.
>>>
>>>> Only the caller knows why it is being called.
>>>
>>> Please find the updated webrev at
>>> http://cr.openjdk.java.net/~mchinnathamb/8027434/webrev.02/
>>> This now uses a static helper to decide.
>>
>>
>> Sorry but I really don't like this version using is_forkmode_vfork. It's
>> awkward to introduce a Linux specific feature into a shared API. But I think
>> the best way to do this would be to introduce a new default parameter to
>> os::fork_and_exec
>>
>> static int fork_and_exec(char *cmd, bool use_vfork_if_available = false);
>>
>> then the impl in o_linux.cpp which use the arg to select fork or vfork. And
>> vmError will do:
>>
>> if (os::fork_and_exec(cmd, true) < 0) {
>>
>> Thanks,
>> David
>>
>>
>>> Regards,
>>> Muthusamy C
>>>
>>> -----Original Message-----
>>> From: David Holmes
>>> Sent: Friday, September 21, 2018 7:27 PM
>>> To: Muthusamy Chinnathambi <muthusamy.chinnathambi at oracle.com>;
>>> hotspot-runtime-dev at openjdk.java.net
>>> Subject: Re: RFR 8027434: "-XX:OnOutOfMemoryError" uses fork instead of
>>> vfork
>>>
>>> Sorry for the delay.
>>>
>>> On 19/09/2018 7:48 AM, Muthusamy Chinnathambi wrote:
>>>>
>>>> Hi David,
>>>>
>>>> Please find the updated webrev at
>>>> http://cr.openjdk.java.net/~mchinnathamb/8027434/webrev.01/
>>>> Instead of creating a static helper, I have moved the decision to the
>>>> fork_and_exec routine itself.
>>>
>>>
>>> That doesn't work. Just because we have an OnOutOfMemoryError handler
>>> set it doesn't mean the call to os::fork_and_exec was made to run that
>>> handler. Only the caller knows why it is being called.
>>>
>>> David
>>>
>>>> Tested with mach5 hs-tier1-4.
>>>>
>>>> Regards,
>>>> Muthusamy C
>>>>
>>>> -----Original Message-----
>>>> From: Muthusamy Chinnathambi
>>>> Sent: Friday, September 14, 2018 12:03 PM
>>>> To: David Holmes <david.holmes at oracle.com>;
>>>> hotspot-runtime-dev at openjdk.java.net
>>>> Subject: RE: RFR 8027434: "-XX:OnOutOfMemoryError" uses fork instead of
>>>> vfork
>>>>
>>>> Hi David,
>>>>
>>>>> please refactor into a static helper function
>>>>
>>>> Sure, I will change and get back after testing.
>>>>
>>>> Regards,
>>>> Muthusamy C
>>>>
>>>> -----Original Message-----
>>>> From: David Holmes
>>>> Sent: Friday, September 14, 2018 11:16 AM
>>>> To: Muthusamy Chinnathambi <muthusamy.chinnathambi at oracle.com>;
>>>> hotspot-runtime-dev at openjdk.java.net
>>>> Subject: Re: RFR 8027434: "-XX:OnOutOfMemoryError" uses fork instead of
>>>> vfork
>>>>
>>>> Hi Muthusamy,
>>>>
>>>> Instead of duplicating all the code - which I think only differs in the
>>>> fork versus vfork line - please refactor into a static helper function
>>>> that takes a boolean vfork and call that from
>>>> fork_and_exec/vfork_and_exec selecting fork or vfork as appropriate.
>>>>
>>>> I'm still mulling over whether there is a cleaner way to do this other
>>>> than introducing a linux only entry to the os class.
>>>>
>>>> Thanks,
>>>> David
>>>>
>>>>
>>>> On 14/09/2018 1:55 PM, Muthusamy Chinnathambi wrote:
>>>>>
>>>>> Hi all,
>>>>>
>>>>> Could you review this fix for JDK-8027434 - to use vfork instead of fork
>>>>> for -XX:OnOutOfMemoryError.
>>>>>
>>>>> See bug comments for more details.
>>>>>
>>>>> Bug URL: https://bugs.openjdk.java.net/browse/JDK-8027434
>>>>> Webrev URL: http://cr.openjdk.java.net/~mchinnathamb/8027434/webrev.00/
>>>>>
>>>>> Tested with mach5 hs-tier1-4.
>>>>>
>>>>> Regards,
>>>>> Muthusamy C
>>>>>
>>