6850720: Allow POSIX_SPAWN to be used for ProcessImpl on Linux

[Thomas Stüfe]

Hi David,

On Fri, Oct 19, 2018 at 2:20 PM David Lloyd <david.lloyd at redhat.com> wrote:
>
> On Fri, Oct 19, 2018 at 1:56 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.
>
> We've gained the same thing that was gained in [1] and for the same
> reason: vfork is deprecated and it is an error to continue to rely
> directly on it.  The only non-deprecated means of using vfork is via
> posix_spawn.

To me this seems not a strong reason. I do not see vfork() ever going away.

My issue with posix_spawn() is that we hand control over how the way
we fork to the libc implementation. That implementation may change. It
did so in the past, for both glibc and muslc (I think muslc is
important beside glibc, see
https://openjdk.java.net/projects/portola/). My point is since we know
exactly what we want we may be better off doing that ourselves.

Also, in the past libc implementators have not been exactly easy to
deal with, so if problems with posix_spawn() come up, it may be
difficult to get them fixed.

>
> And, posix_spawn is not the game of roulette that you make it out to
> be.  To quote the manual page:
>
> "The child process is created using vfork(2) instead of fork(2) when
> either of the following is true:
>
> *  the spawn-flags element of the attributes object pointed to by
> attrp contains the GNU-specific flag POSIX_SPAWN_USEVFORK; or
>
> *  file_actions is NULL and the spawn-flags element of the attributes
> object  pointed  to  by  attrp  does  not  contain
> POSIX_SPAWN_SETSIGMASK,  POSIX_SPAWN_SETSIGDEF,
> POSIX_SPAWN_SETSCHEDPARAM, POSIX_SPAWN_SETSCHEDULER,
> POSIX_SPAWN_SETPGROUP, or POSIX_SPAWN_RESETIDS.
>
> In other words, vfork(2) is used if the caller requests it, or if
> there is no cleanup expected in the child before it exec(3)s the
> requested file."
>
> It is true that we aren't explicitly requesting it, however, in our
> case file_actions is NULL and attrp is also NULL, so posix_spawn will
> use vfork* on Linux.
>
> * Actually neither fork nor vfork use anything but the clone()
> syscall.  To quote the sources, "The Linux implementation of
> posix_spawn{p} uses the clone syscall directly with CLONE_VM and
> CLONE_VFORK flags and an allocated stack."
>
> > Note that glibc has a flag to force always vfork behavior:
> > POSIX_SPAWN_USEVFORK.
>
> Sure, but I don't really see this as necessary if glibc is already
> following the vfork-like path.  Another thing to know is that at least
> in the 2.26 sources I have checked out, the POSIX_SPAWN_USEVFORK flag
> is completely ignored.  See also [2].
>
> [1] https://bugs.openjdk.java.net/browse/JDK-8161360
> [2] https://github.com/bminor/glibc/commit/ccfb2964726512f6669fea99a43afa714e2e6a80
>
> --
> - DML


You may be wrong about [2]. The relevant implementation in glibc seems
to be this:

https://github.com/tstuefe/glibc/blob/master/sysdeps/unix/sysv/linux/spawni.c

and this seems to be the relevant change:

https://github.com/tstuefe/glibc/commit/9ff72da471a509a8c19791efe469f47fa6977410

( [2] is the "posix implementation". I am not sure what that is, maybe
for Hurd?)

So, glibc posix_spawn() uses clone with CLONE_VM and CLONE_VFORK.

Interesting: if I understand the code right, they mitigate part of the
dangers of vfork aka CLONE_VM by allocating a new separate stack for
the child.

---

Lets see what muslc does:

https://github.com/esmil/musl/blob/master/src/process/posix_spawn.c

So, they do exactly the same (I think the musl folks did this before
glibc switched to clone). use clone(CLONE_VM|CLONE_VFORK).

Hm okay. In conjunction with jspawnhelper this may work fine... I have
to mull this over a bit more.

--

One concern for me is that the change in the glibc is quite recent -
2016 - and that there may be many old glibc variants in the field
installed which still use fork(). On those installations, switching
the java vm to suddenly use posix_spawn may introduce regressions.

My main concern - reliance on glibc developers for something we should
better do ourselves - is unshaken, but lets see what others say.

Kind Regards, Thomas