RFR(s): 8212828: (process) Change the Process launch mechanism default on Linux to be posix_spawn

[Thomas Stüfe]

Hi Roger, Martin,

next version:

http://cr.openjdk.java.net/~stuefe/webrevs/8213192--(process)-change-the-process-launch-mechanism-default-on-linux-to-be-posix_spawn/webrev.02/webrev

- did massage the comment in ProcessImpl.c
- made the test more resilient by scanning for the strace tool and by
silently ignoring all problems stemming from strace or the payload binary
not being there. The test now only fails if the forks were successully
executed but it does not seem to use posix-spawn.
- added output to the test by printing the "interesting" lines of the
strace output. Note that this filtering is not really sophisticated and
will show all thread related clone() calls as well:

614   [pid 12447] <... clone resumed> child_stack=0x7fe00c4baff0,
flags=CLONE_VM|CLONE_FS|CLONE_FILES|CLONE_SIGHAND|CLONE_THREAD|CLONE_SYSVSEM|CLONE_SETTLS|CLONE_PARENT_SETTID|CLONE_CHILD_CLEARTID,
parent_tidptr=0x7fe00c4bb9d0, tls=0x7fe00c4bb700,
child_tidptr=0x7fe00c4bb9d0) = 12452
646   [pid 12447] clone(/usr/bin/strace: Process 12453 attached
649   [pid 12447] <... clone resumed> child_stack=0x7fe00c3b9ff0,
flags=CLONE_VM|CLONE_FS|CLONE_FILES|CLONE_SIGHAND|CLONE_THREAD|CLONE_SYSVSEM|CLONE_SETTLS|CLONE_PARENT_SETTID|CLONE_CHILD_CLEARTID,
parent_tidptr=0x7fe00c3ba9d0, tls=0x7fe00c3ba700,
child_tidptr=0x7fe00c3ba9d0) = 12453
....

I am sure this could be made more intelligent but lets keep it simple for
now.

- I removed the helperPath() methods Roger mentioned, they are not needed
anymore.

@Martin: I like the -e signal=none -e trace=process idea but I'm not sure
if all versions of strace support these options. I think the strace output
is small enough for this small use case, some kB only.

Cheers, Thomas





On Thu, Feb 7, 2019 at 6:01 PM Thomas Stüfe <thomas.stuefe at gmail.com> wrote:

> Hi all,
>
> second version, including the updated comment in ProcessImpl.c Martin
> requested:
>
>
> http://cr.openjdk.java.net/~stuefe/webrevs/8213192--(process)-change-the-process-launch-mechanism-default-on-linux-to-be-posix_spawn/webrev.01/webrev/index.html
>
> @Roger: thanks for feeding this into your tests. I still try to get it to
> run thru jdk-submit, but that seems to be stuck again..
>
> Cheers, Thomas
>
>
>
>
>
> On Wed, Feb 6, 2019 at 10:29 AM Thomas Stüfe <thomas.stuefe at gmail.com>
> wrote:
>
>> Hi all
>>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8213192
>> webrev:
>> http://cr.openjdk.java.net/~stuefe/webrevs/8213192--(process)-change-the-process-launch-mechanism-default-on-linux-to-be-posix_spawn/webrev.00/webrev/index.html
>>
>> (@Roger: I hope you do not mind? The bug is assigned to you but since I
>> happened to play around with posix_spawn I prepared this webrev. If you
>> rather do this change, that is fine and I will leave it to you.)
>>
>> When we added the possibility to use posix_spawn as underlying
>> implementation for Runtime.exec() on Linux with
>> https://bugs.openjdk.java.net/browse/JDK-8212828, we agreed to keep
>> VFORK as default until work on 13 starts. So now would be a good time to
>> switch the default to posix_spawn to get a good testing window. Note that
>> at SAP we run our VMs internally with posix_spawn as default since some
>> months and have not seen problems.
>>
>> As for the fix, I added a test which tests that the default is indeed
>> posix_spawn - not sure whether this is overdoing it though. Also, I use
>> strace for the test, and /bin/true, and while strace is usually available
>> and reachable by path resolution, I am afraid on some test machines it may
>> not. What do you think, should I leave the test out?
>>
>> The fix ran through all java/lang/ProcessBuilder jtreg tests ok.
>>
>> Thanks, Thomas
>>
>>