RFR: JDK-8212828 Allow POSIX_SPAWN to be used for ProcessImpl on Linux

[David Lloyd]

I'm not really sure TBH.  I did some manual testing; beyond that, in a
way, the point of this change is to *get* more real-world testing.  I
haven't yet found any existing tests which try out the various process
creation options for a given platform, however.

Maybe an idea would be to have a variation on
test/jdk/java/lang/ProcessBuilder/BigFork.java which tries all of the
LaunchMechanism options (other than FORK, which I believe is known to
likely fail with this test) which are available on the current arch?
Also there's test/jdk/java/lang/ProcessBuilder/Basic.java which could
possibly be adapted to run multiple times as well (and maybe also to
cover FORK which is supported for many archs but AFAICT not tested on
archs where there are other options, i.e. all of them).  But I'm not
really sure how to make that happen with the OpenJDK test
infrastructure.

As for build changes, no I do not believe any are needed, though I am
not able to build on non-Linux platforms to be sure; I was hoping to
get a run through jdk-submit to verify that but I don't seem to have
access to that either, having no particular status in the OpenJDK
project other than sometime contributor.  Martin did suggest that
maybe the spawn native code could be modified to use config-based
macros (e.g. HAS_SPAWN_H) instead of the current platform-based
detection but I feel that's out of scope for this little change.
On Tue, Oct 23, 2018 at 2:53 PM Roger Riggs <Roger.Riggs at oracle.com> wrote:
>
> Hi David, et.al.
>
> What would be the rest of the plan for testing?
> Usually, changes come with tests and a plan.
> What build parameters are needed to run a full set of tests with the change?
>
> Are there build changes needed?
>
> Thanks, Roger
>
>
> On 10/23/2018 03:26 PM, David Lloyd wrote:
> > My plans to try jdk/submit have fallen through unfortunately, as I
> > cannot seem to gain direct or indirect access to that system.  So I
> > guess I'm looking for any reviews on this patch now.  Thomas has
> > volunteered to sponsor.
> >
> > Thanks.
> >
> > On Tue, Oct 23, 2018 at 10:49 AM Thomas Stüfe <thomas.stuefe at gmail.com> wrote:
> >> Here you go:
> >>
> >> https://bugs.openjdk.java.net/browse/JDK-8212828
> >>
> >> If noone else steps in, I can sponsor the change for you.
> >>
> >> Cheers, Thomas
> >> On Tue, Oct 23, 2018 at 4:19 PM David Lloyd <david.lloyd at redhat.com> wrote:
> >>> Sure.  I don't have any tracking information on the bugreport one I
> >>> submitted, but if you can track that down and promote it, it would
> >>> save you some typing.  Otherwise whatever you can do would be great,
> >>> thanks.
> >>> On Tue, Oct 23, 2018 at 9:02 AM Thomas Stüfe <thomas.stuefe at gmail.com> wrote:
> >>>> Oh, I can open a bug report on JBS for you. Should I?
> >>>>
> >>>> (Now I understand the "reuse bug id").
> >>>>
> >>>>
> >>>> On Tue, Oct 23, 2018 at 3:18 PM David Lloyd <david.lloyd at redhat.com> wrote:
> >>>>> I've submitted a bug report via bugreport.java.com.  If/when it gets
> >>>>> promoted to a proper JIRA with an issue number, I'll see if I can put
> >>>>> the patch up on jdk/submit.
> >>>>> On Thu, Oct 18, 2018 at 4:42 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".
> >
> >
>


-- 
- DML