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

[David Holmes]

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