[8u60] RFR: 8078470: [Linux] Replace syscall use in os::fork_and_exec with glibc fork() and execve()
Dmitry Samersoff
dmitry.samersoff at oracle.com
Tue Apr 28 08:06:36 UTC 2015
David,
http://cr.openjdk.java.net/~dholmes/8078470/webrev.v2/
Looks good for me. Not a *R*eviewer.
-Dmitry
On 2015-04-28 07:50, David Holmes wrote:
> Ping: still need a (R)eviewer please.
>
> Thanks,
> David
>
> On 25/04/2015 7:45 AM, David Holmes wrote:
>> Hi Thomas,
>>
>> Thanks for looking at this.
>>
>> Note to readers: Still need a (R)eviewer for this please.
>>
>> On 24/04/2015 7:00 PM, Thomas Stüfe wrote:
>>> Hi David,
>>>
>>> Unfortunately printing out errno in VMError will not work so well, as it
>>> is not clear if it is from fork() or execve(), and it is vulnerable
>>> against future changes of os::fork_and_exec(). Also, will not do
>>> anything for you on Windows.
>>
>> I thought about doing something inside fork_and_exec but decided against
>> it for a couple of reasons:
>>
>> 1. Needs to be done for every platform individually.
>> 2. Far from clear which mechanism would be appropriate given the
>> different contexts in which it can be used - which is also why I did not
>> do something for its use when debugging.
>>
>> As for Windows ... not sure what it will do but I was following existing
>> practice - see os_windows.cpp and perfMemory_windows.cpp.
>>
>>> I do not see a nice solution. Returning errno will not work, or be ugly,
>>> because you'd need to return GetLastError for windows, which has a
>>> different semantic, and the caller must remember that.
>>
>> And a return value, unless encoded, won't tell you which of fork and
>> execve failed.
>>
>>> On Solaris, os::fork_and_exec() prints a warning if PrintWarning() is
>>> on, which in my opinion does not help. That printout is very verbose and
>>> you have to remember setting it before starting the process.
>>>
>>> In some places in error handling (e.g. "check_dump_limits()") we return
>>> a string in a caller provided buffer describing the error which then is
>>> printed into the error logs. So, that may be a pragmatic solution,
>>> especially because we already have the scratch buffer in
>>> VMError::report_and_die().
>>>
>>> On the other hand, in almost any os::... function we just swallow error
>>> details, as we did in os::fork_and_exec() before, so maybe we just leave
>>> it at this.
>>
>> I'll leave what I have now, pending further comments from anyone. It is
>> better than nothing I think, and would have saved me a bit of time when
>> I first encountered the problem with the missing syscall.
>>
>>> Otherwise, change looks fine to me.
>>
>> Thanks,
>> David
>>
>>> ..Thomas
>>>
>>>
>>>
>>>
>>> On Fri, Apr 24, 2015 at 4:03 AM, David Holmes <david.holmes at oracle.com
>>> <mailto:david.holmes at oracle.com>> wrote:
>>>
>>> Hi Thomas,
>>>
>>> On 23/04/2015 6:00 PM, Thomas Stüfe wrote:
>>>
>>> On Thu, Apr 23, 2015 at 9:50 AM, David Holmes
>>> <david.holmes at oracle.com <mailto:david.holmes at oracle.com>
>>> <mailto:david.holmes at oracle.com
>>> <mailto:david.holmes at oracle.com>>> wrote:
>>> On 23/04/2015 5:24 PM, Thomas Stüfe wrote:
>>> - would it be possible to use vfork() instead of
>>> fork()? Might
>>> increase
>>> the chance of fork() succeeding in out-of-memory
>>> scenarios. The way
>>> fork/execve is used here is simple enough for vfork().
>>>
>>> I'm reluctant to use vfork() as it is an unknown. The use
>>> of fork()
>>> has been trialled by Google with no problem.
>>>
>>> I understand.
>>>
>>> We use vfork() by default on Linux and HP-UX since 3-4 years for
>>> Runtime.exec() without problems (but our implementation differs
>>> wildly
>>> from the standard one). But then, Runtime.exec does not get
>>> called in
>>> error situations, so the conditions may be different for
>>> os::fork_and_exec().
>>>
>>>
>>> Right - os::fork_and_exec is potentially a lot more fragile given
>>> the potential execution context. Which prompted me to add a second
>>> test for the general OnError handling case. I also moved the tests
>>> to runtime/ErrorHandling to match the directory that exists in
>>> JDK 9.
>>>
>>> Updated webrev:
>>>
>>> http://cr.openjdk.java.net/~dholmes/8078470/webrev.v2/
>>>
>>> Thanks,
>>> David
>>>
>>>
>>> - would it be possible to print out errno in case fork
>>> fails?
>>>
>>>
>>> Sure.
>>>
>>>
>>> Thanks!
>>>
>>> ..Thomas
>>>
>>> Thanks,
>>> David
>>>
>>> Regards, Thomas
>>>
>>>
>>> On Thu, Apr 23, 2015 at 8:40 AM, David Holmes
>>> <david.holmes at oracle.com
>>> <mailto:david.holmes at oracle.com> <mailto:david.holmes at oracle.com
>>> <mailto:david.holmes at oracle.com>>
>>> <mailto:david.holmes at oracle.com
>>> <mailto:david.holmes at oracle.com>
>>>
>>> <mailto:david.holmes at oracle.com
>>> <mailto:david.holmes at oracle.com>>>> wrote:
>>>
>>> webrev:
>>> http://cr.openjdk.java.net/~dholmes/8078470/webrev/
>>>
>>> bug:
>>> https://bugs.openjdk.java.net/browse/JDK-8078470
>>>
>>> For historical reasons, dating back to
>>> LinuxThreads,
>>> os::fork_and_exec uses direct syscalls to perform
>>> fork and
>>> execve
>>> functions. The fork syscall has been deprecated on
>>> linux
>>> for quite
>>> some time and some distributions are now
>>> shipping with
>>> deprecated
>>> syscalls removed - this results in a ENOSYS error
>>> and the
>>> "OnError"
>>> commands that utilize os::fork_and_exec no longer
>>> work.
>>>
>>> The problem was discussed here:
>>>
>>>
>>> http://mail.openjdk.java.net/pipermail/hotspot-dev/2015-April/017916.html
>>>
>>>
>>> It seems the concerns surrounding direct use of
>>> glibc
>>> fork() and
>>> exec() are no longer legitimate and Martin
>>> Buchholz reports
>>> that
>>> Google replaced the syscalls with glibc calls some
>>> time ago
>>> and have
>>> not had any problems. So we propose to apply the
>>> same change
>>> uniformly on Linux.
>>>
>>> This is initially going into 8u60 via jdk8u-hs-dev
>>> due to time
>>> constraints on the 8u60 schedule, and an internal
>>> limitation
>>> preventing me from pushing this to 9 right now.
>>> But it will be
>>> "backported" to 9 ASAP.
>>>
>>> Thanks,
>>> David
>>>
>>>
>>>
>>>
--
Dmitry Samersoff
Oracle Java development team, Saint Petersburg, Russia
* I would love to change the world, but they won't give me the sources.
More information about the hotspot-dev
mailing list