[8u60] RFR: 8078470: [Linux] Replace syscall use in os::fork_and_exec with glibc fork() and execve()

David Holmes david.holmes at oracle.com
Tue Apr 28 04:50:26 UTC 2015


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
>>
>>
>>
>>


More information about the hotspot-dev mailing list