6850720: Allow POSIX_SPAWN to be used for ProcessImpl on Linux

Mon Oct 22 19:41:39 UTC 2018

I ran some tests on my local I ran a number of tests on various
machines and architectures, all with glibc variants older than 2.24,
and straced them, and they all used vfork() internally.

That made me curious, and I dug into the glibc sources and examined
the history. If I understand this correctly:

1) before glibc 2.4, posix_spawn() used just fork()

2) between glibc 2.4 and glibc 2.23, posix_spawn uses either fork()
and vfork(), as still described in the manpage today. The relevant
coding (sysdeps/posix/spawni.c) looks like this:

 93   /* Do this once.  */
 94   short int flags = attrp == NULL ? 0 : attrp->__flags;
 95
 96   /* Generate the new process.  */
 97   if ((flags & POSIX_SPAWN_USEVFORK) != 0
 98       /* If no major work is done, allow using vfork.  Note that we
 99          might perform the path searching.  But this would be done by
100          a call to execvp(), too, and such a call must be OK according
101          to POSIX.  */
102       || ((flags & (POSIX_SPAWN_SETSIGMASK | POSIX_SPAWN_SETSIGDEF
103                     | POSIX_SPAWN_SETSCHEDPARAM | POSIX_SPAWN_SETSCHEDULER
104                     | POSIX_SPAWN_SETPGROUP | POSIX_SPAWN_RESETIDS)) == 0
105           && file_actions == NULL))
106     new_pid = __vfork ();
107   else
108     new_pid = __fork ();
109
1

So, for our use case this means it always uses vfork() since we do not
pass attributes nor actions, as David said before. We do not have to
since we run the jspawnhelper which does all that for us at the
expense of a second exec() call. This looks okay for me.

3) and since 2.24 we use the new linux variant in
sysdeps/unix/sysv/linux/spawni.c

365   new_pid = CLONE (__spawni_child, STACK (stack, stack_size), stack_size,
366                    CLONE_VM | CLONE_VFORK | SIGCHLD, &args);

which is okay for me too. In fact this is better since the child
process runs on its own stack before the exec().

---

Okay, seeing that (2) and (3) look fine and (1) only affects glibc
versions older than 2.4, which came out in 2006, I admit this is okay
and I worry too much. So I'll be quiet now :)

Best Regards, Thomas
Ubuntu 16.4 and various machines at work, with varying glibc versions as old as
On Fri, Oct 19, 2018 at 8:55 AM Thomas Stüfe <thomas.stuefe at gmail.com> wrote:
>
> Hi David,
>
> (for convenience, here the old thread:
> http://mail.openjdk.java.net/pipermail/core-libs-dev/2018-September/055333.html)
>
> I would rather not expose a third way to spawn to the end user. As a
> test switch for developers, this seems fine.
>
> AFAIK (and someone please correct me if I am wrong) posix_spawn is not
> the magical third way beside fork and vfork, it is just a wrapper
> around fork/vfork and exec. Especially posix_spawn(), if using fork(),
> will still have the same pitfalls raw fork() has - quasi-random ENOMEM
> if we hit the overcommit heuristic, and inferior performance compared
> to vfork. So you have nothing gained, you have just relegated the
> fork/vfork decision down to a different layer, one which you have
> little control over. And since that fork/vfork decision is kind of
> important, you need that contol.
>
> Note that glibc has a flag to force always vfork behavior:
> POSIX_SPAWN_USEVFORK. The question is, how tight do we bind us to
> GNU-only features, since we may run on linux with a different libc,
> e.g. on Alpine we use muslc.
>
> My proposal, vfork() + the exec-twice-trick, is IMHO the best way to
> go. That exec-twice trick takes the sting away from vfork. After
> Martin made supportive noises in the other mail thread, I have planned
> in some time later this year to produce a patch for this.
>
> --
>
> However: I just realized that my proposal (vfork() + exec-twice-trick)
> is very close, implementation wise, to the (posix_spawn() +
> POSIX_SPAWN_USEVFORK + jspawnhelper). The jspawnhelper already
> implements the exec-twice trick, if only for different reasons than to
> get away from vfork dangers. Advantage of this implementation would be
> that very little code would have to change. Disadvantage, in my eyes,
> is that we bind us to glibc, and an aesthetic side: calling an OS API
> but needing it to work in one specific way feels like programming with
> mechanical gloves...
>
> Thanks & Best Regards, Thomas
>
> (Copying Martin, I'm curious what he thinks)
>
> --
>
> On Thu, Oct 18, 2018 at 11:43 PM David Lloyd <david.lloyd at redhat.com> wrote:
> >
> > The issue 6850720 isn't _exactly_ to use POSIX_SPAWN for process
> > launching on Linux, but it's the closest I could find out of what are
> > really a surprisingly large number of issues that refer to posix_spawn
> > in one way or another relating to ProcessImpl.  There's a different
> > issue to move from vfork to posix_spawn on Solaris, but I wasn't sure
> > if that one was quite right to hang this off of.  Maybe it should be
> > yet another issue of its own.
> >
> > Anyway: this is a follow-up to the email thread entitled "Runtime.exec
> > : vfork() concerns and a fix proposal", where it was casually
> > mentioned that maybe posix_spawn could become an option on Linux,
> > whereafter it could be thoroughly tested by brave individuals and
> > eventually maybe become the default on that platform, obsoleting the
> > vfork support for good.
> >
> > The following patch does just that.  I've tested it launching a
> > multi-process WildFly instance a bunch of times, in conjunction with
> > the conveniently existent "jdk.lang.Process.launchMechanism" property,
> > and nothing exploded so here it is.  The usual deal with git patches:
> > apply directly through "patch -p1".
> >
> > commit f0eb9ff7c46dff76f91160491fcca0eb25d0ab95
> > Author: David M. Lloyd <david.lloyd at redhat.com>
> > Date:   Thu Oct 18 15:56:37 2018 -0500
> >
> >     [JDK-6850720] Enable POSIX_SPAWN as an option for child process
> > creation on Linux
> >
> > diff --git a/make/launcher/Launcher-java.base.gmk
> > b/make/launcher/Launcher-java.base.gmk
> > index 0ce0287d2be..c28fe42d102 100644
> > --- a/make/launcher/Launcher-java.base.gmk
> > +++ b/make/launcher/Launcher-java.base.gmk
> > @@ -84,7 +84,7 @@ endif
> >
> >  ################################################################################
> >
> > -ifneq ($(findstring $(OPENJDK_TARGET_OS), macosx solaris aix), )
> > +ifneq ($(findstring $(OPENJDK_TARGET_OS), macosx solaris aix linux), )
> >    $(eval $(call SetupJdkExecutable, BUILD_JSPAWNHELPER, \
> >        NAME := jspawnhelper, \
> >        SRC := $(TOPDIR)/src/$(MODULE)/unix/native/jspawnhelper, \
> > diff --git a/src/java.base/unix/classes/java/lang/ProcessImpl.java
> > b/src/java.base/unix/classes/java/lang/ProcessImpl.java
> > index 368a4f7380b..959e50dfecd 100644
> > --- a/src/java.base/unix/classes/java/lang/ProcessImpl.java
> > +++ b/src/java.base/unix/classes/java/lang/ProcessImpl.java
> > @@ -89,7 +89,7 @@ final class ProcessImpl extends Process {
> >
> >      private static enum Platform {
> >
> > -        LINUX(LaunchMechanism.VFORK, LaunchMechanism.FORK),
> > +        LINUX(LaunchMechanism.VFORK, LaunchMechanism.POSIX_SPAWN,
> > LaunchMechanism.FORK),
> >
> >          BSD(LaunchMechanism.POSIX_SPAWN, LaunchMechanism.FORK),
> >
> > diff --git a/src/java.base/unix/native/libjava/ProcessImpl_md.c
> > b/src/java.base/unix/native/libjava/ProcessImpl_md.c
> > index 533584fdb7a..6869a64f2cc 100644
> > --- a/src/java.base/unix/native/libjava/ProcessImpl_md.c
> > +++ b/src/java.base/unix/native/libjava/ProcessImpl_md.c
> > @@ -44,7 +44,7 @@
> >  #include <signal.h>
> >  #include <string.h>
> >
> > -#if defined(__solaris__) || defined(_ALLBSD_SOURCE) || defined(_AIX)
> > +#if defined(__solaris__) || defined(_ALLBSD_SOURCE) || defined(_AIX)
> > || defined(__linux__)
> >  #include <spawn.h>
> >  #endif
> >
> > @@ -390,7 +390,7 @@ forkChild(ChildStuff *c) {
> >      return resultPid;
> >  }
> >
> > -#if defined(__solaris__) || defined(_ALLBSD_SOURCE) || defined(_AIX)
> > +#if defined(__solaris__) || defined(_ALLBSD_SOURCE) || defined(_AIX)
> > || defined(__linux__)
> >  static pid_t
> >  spawnChild(JNIEnv *env, jobject process, ChildStuff *c, const char
> > *helperpath) {
> >      pid_t resultPid;
> > @@ -489,7 +489,7 @@ startChild(JNIEnv *env, jobject process,
> > ChildStuff *c, const char *helperpath)
> >  #endif
> >        case MODE_FORK:
> >          return forkChild(c);
> > -#if defined(__solaris__) || defined(_ALLBSD_SOURCE) || defined(_AIX)
> > +#if defined(__solaris__) || defined(_ALLBSD_SOURCE) || defined(_AIX)
> > || defined(__linux__)
> >        case MODE_POSIX_SPAWN:
> >          return spawnChild(env, process, c, helperpath);
> >  #endif