This is very good. Approved. But as always I have review suggestions typos: ithe Preceede => Precede
Drop "the" How does the glibc implement posix_spawn? How does the muslc implement posix_spawn? parents => the parent CLONE_VFORK means parents waits until we exec, as with (2) an own => a separate we pass an own stack for the child to run on Did you mean tlrd => TL;DR ? https://en.wikipedia.org/wiki/TL;DR You can drop the pre-2004 part from the TL;DR 164 * tlrd: calling posix_spawn(3) for glibc 165 * (1) < 2.4 would expose us to memory overcommit problems. But this glibc is The test is still too brittle for my taste. I would check for the existence of /usr/bin/strace (and /bin/true !) and quietly skip the test if not found. I don't like uninformative prints System.out.print("I'm the child. Will fork /bin/true now..."); Instead I might be truly interested in whether the strace output contains fork|vfork|clone fyi I have a wrapper around strace for process-related syscalls #!/bin/bash /usr/bin/strace -f -v -s 256 -e signal=none -e trace=process "$@" On Thu, Feb 7, 2019 at 9:01 AM Thomas Stüfe <thomas.stu...@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.stu...@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 >> >>
