Re: PING: RFR(s): (new approach) 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error

2019-06-06 Thread Florian Weimer
* Thomas Stüfe: > Hi Florian, > > Interesting, but in this case it is not posix_spawn, but plain > fork(). The VM does this, the exec calls come from us, not the > libc. See childproc.c . Ah. Others have unearthed the genealogy. Thanks for that. I wonder if in a Java context, things are

Re: PING: RFR(s): (new approach) 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error

2019-06-05 Thread Roger Riggs
+1, Its essentially (mostly) dead code, but there is a risk of removing it. Queue it up for the beginning of JDK 14 if its worth even touching it. Better things to spend time on Roger On 06/05/2019 10:58 AM, Martin Buchholz wrote: On Wed, Jun 5, 2019 at 7:51 AM David Lloyd

Re: PING: RFR(s): (new approach) 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error

2019-06-05 Thread Martin Buchholz
On Wed, Jun 5, 2019 at 7:51 AM David Lloyd wrote: > If we're talking just Linux though, has this *ever* been an issue? You've > been able to call execve on a shell script since at least 2002 as far as I > can tell from a quick search. Maybe this should be conditional so that it > can be

Re: PING: RFR(s): (new approach) 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error

2019-06-05 Thread David Lloyd
https://www.unix.com/man-page/redhat/2/execve/ is from 1997. On Wed, Jun 5, 2019 at 9:50 AM David Lloyd wrote: > If we're talking just Linux though, has this *ever* been an issue? You've > been able to call execve on a shell script since at least 2002 as far as I > can tell from a quick

Re: PING: RFR(s): (new approach) 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error

2019-06-05 Thread David Lloyd
If we're talking just Linux though, has this *ever* been an issue? You've been able to call execve on a shell script since at least 2002 as far as I can tell from a quick search. Maybe this should be conditional so that it can be excluded on known-good platforms? On Wed, Jun 5, 2019 at 9:40 AM

Re: PING: RFR(s): (new approach) 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error

2019-06-05 Thread Martin Buchholz
https://pubs.opengroup.org/onlinepubs/9699919799/functions/exec.html """Historically, there have been two ways that implementations can exec shell scripts. One common historical implementation is that the execl(), execv(), execle(), and execve() functions return an [ENOEXEC] error for any file

Re: PING: RFR(s): (new approach) 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error

2019-06-05 Thread Thomas Stüfe
On Wed, Jun 5, 2019 at 8:37 AM Martin Buchholz wrote: > > > On Tue, Jun 4, 2019 at 9:50 PM Thomas Stüfe > wrote: > >> >> I would vote for keeping the name as it is: I would have to change a >> number of places, and when in doubt I'd like to keep the changes minimal to >> make backporting future

Re: PING: RFR(s): (new approach) 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error

2019-06-05 Thread Thomas Stüfe
Hi David, You are right. I am pretty sure there are ancient historical reasons for doing this by hand instead of letting exec() deal with scripts itself. Martin or Roger may know more. But I think right now this is more of a compatibility concern - changing this behavior has the risk of breaking

Re: PING: RFR(s): (new approach) 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error

2019-06-05 Thread Martin Buchholz
On Tue, Jun 4, 2019 at 9:50 PM Thomas Stüfe wrote: > > I would vote for keeping the name as it is: I would have to change a > number of places, and when in doubt I'd like to keep the changes minimal to > make backporting future changes easier. > OK - author gets to cast deciding vote!

Re: PING: RFR(s): (new approach) 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error

2019-06-04 Thread Thomas Stüfe
On Wed, Jun 5, 2019 at 5:06 AM Martin Buchholz wrote: > > > On Tue, Jun 4, 2019 at 2:01 PM Roger Riggs wrote: > >> Hmm. Can_JOHNNY_EXEC naming is a bit quirky, not intuitive... >> > > I'm the original author of that joke^H deeply evocative symbol name, so I > might be biased. > > I like the

Re: PING: RFR(s): (new approach) 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error

2019-06-04 Thread Martin Buchholz
On Tue, Jun 4, 2019 at 2:01 PM Roger Riggs wrote: > Hmm. Can_JOHNNY_EXEC naming is a bit quirky, not intuitive... > I'm the original author of that joke^H deeply evocative symbol name, so I might be biased.

Re: PING: RFR(s): (new approach) 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error

2019-06-04 Thread Roger Riggs
Hmm.  Can_JOHNNY_EXEC naming is a bit quirky, not intuitive... On 06/04/2019 04:52 PM, Martin Buchholz wrote: On Tue, Jun 4, 2019 at 10:40 AM Thomas Stüfe > wrote: On Tue, Jun 4, 2019 at 7:21 PM Martin Buchholz mailto:marti...@google.com>> wrote:

Re: PING: RFR(s): (new approach) 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error

2019-06-04 Thread Martin Buchholz
On Tue, Jun 4, 2019 at 10:40 AM Thomas Stüfe wrote: > > > On Tue, Jun 4, 2019 at 7:21 PM Martin Buchholz > wrote: > >> >> Unfortunately, it does make the subprocess implementation even >> more complicated, since now "fail" is used to communicate success as well >> as failure. Which probably

Re: PING: RFR(s): (new approach) 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error

2019-06-04 Thread Thomas Stüfe
Hi Florian, Interesting, but in this case it is not posix_spawn, but plain fork(). The VM does this, the exec calls come from us, not the libc. See childproc.c . Cheers, Thomas’ On Tue 4. Jun 2019 at 22:42, Florian Weimer wrote: > * Thomas Stüfe: > > > Then I ran an strace over it and saw

Re: PING: RFR(s): (new approach) 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error

2019-06-04 Thread Florian Weimer
* Thomas Stüfe: > Then I ran an strace over it and saw this: > > 5332 [pid 3911] execve("./sleep2", ["./sleep2"], [/* 78 vars */] ...> > > .. > 5342 [pid 3911] <... execve

Re: PING: RFR(s): (new approach) 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error

2019-06-04 Thread David Lloyd
In this case, going from what the execve(2) Linux man page says (and Mac seems to agree), if the script were executable it would have a '#!' interpreter string and the kernel would execute it anyway, would it not? What case would be solved by explicitly starting a shell? On Tue, Jun 4, 2019 at

Re: PING: RFR(s): (new approach) 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error

2019-06-04 Thread Thomas Stüfe
Thank you for clarifying, and for the review. Cheers, Thomas On Tue, Jun 4, 2019 at 9:14 PM Roger Riggs wrote: > Hi Thomas, > > If the argument is not an .exe, then it may be a command shell script > (.sh, etc.) > Only the system knows it is not an exe (errno == ENOEXEC), so it then > passes

Re: PING: RFR(s): (new approach) 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error

2019-06-04 Thread Roger Riggs
Hi Thomas, If the argument is not an .exe, then it may be a command shell script  (.sh, etc.) Only the system knows it is not an exe (errno == ENOEXEC), so it then passes it as the first argument to /bin/sh that will handle the shell files. And yes, count me as a Reviewer and reviewed.

Re: PING: RFR(s): (new approach) 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error

2019-06-04 Thread Thomas Stüfe
On Tue, Jun 4, 2019 at 7:25 PM Martin Buchholz wrote: > > > On Tue, Jun 4, 2019 at 8:09 AM Roger Riggs wrote: > >> Hi Thomas, >> >> A minor concern is the impact of the extra write and read that can cause >> rescheduling >> of the parent and child processes. But that's probably in the noise >>

Re: PING: RFR(s): (new approach) 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error

2019-06-04 Thread Thomas Stüfe
On Tue, Jun 4, 2019 at 7:21 PM Martin Buchholz wrote: > Hi Thomas, > > Thanks - I agree with the approach. > > Unfortunately, it does make the subprocess implementation even > more complicated, since now "fail" is used to communicate success as well > as failure. Which probably results in some

Re: PING: RFR(s): (new approach) 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error

2019-06-04 Thread Martin Buchholz
On Tue, Jun 4, 2019 at 8:09 AM Roger Riggs wrote: > Hi Thomas, > > A minor concern is the impact of the extra write and read that can cause > rescheduling > of the parent and child processes. But that's probably in the noise > compared to the > real work of exec. It would raise the complexity

Re: PING: RFR(s): (new approach) 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error

2019-06-04 Thread Martin Buchholz
Hi Thomas, Thanks - I agree with the approach. Unfortunately, it does make the subprocess implementation even more complicated, since now "fail" is used to communicate success as well as failure. Which probably results in some comments becoming stale. Can we come up with a better name for

Re: PING: RFR(s): (new approach) 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error

2019-06-04 Thread Thomas Stüfe
Hi Roger, thanks for the feedback. Please find my answers inline. On Tue, Jun 4, 2019 at 5:09 PM Roger Riggs wrote: > Hi Thomas, > > A minor concern is the impact of the extra write and read that can cause > rescheduling > of the parent and child processes. But that's probably in the noise >

Re: PING: RFR(s): (new approach) 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error

2019-06-04 Thread Roger Riggs
Hi Thomas, A minor concern is the impact of the extra write and read that can cause rescheduling of the parent and child processes.  But that's probably in the noise compared to the real work of exec.  It would raise the complexity quite a bit/too much to code a single read in the parent

PING: RFR(s): (new approach) 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error

2019-06-04 Thread Thomas Stüfe
Hi all, may I please have reviews/opinions on this fix? Fix has been live in our test landscape since some weeks. If we do not want this fix to be in JDK13, we may want to revert the posix_spawn-by-default-on-Linux change. Thanks, Thomas On Mon, May 20, 2019 at 4:15 PM Thomas Stüfe wrote:

Re: RFR(s): (new approach) 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error

2019-05-22 Thread Thomas Stüfe
Any technical input is welcome, Reviewer or not. If a non-Reviewer sees valid reasons for or against a patch, that is good to know. Cheers, Thomas On Wed, May 22, 2019 at 4:41 PM David Lloyd wrote: > I'm in favor of what the change is meant to accomplish. I haven't had > time to analyze the

Re: RFR(s): (new approach) 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error

2019-05-22 Thread David Lloyd
I'm in favor of what the change is meant to accomplish. I haven't had time to analyze the change in detail, and I may not get time to do so. But I'm not a reviewer in any case, so maybe that doesn't matter too much. On Wed, May 22, 2019 at 1:16 AM Thomas Stüfe wrote: > > Ping... > > Guys, I

Re: RFR(s): (new approach) 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error

2019-05-22 Thread Thomas Stüfe
No problems in tier 1 tests (jdk-submit). ..Thomas On Wed, May 22, 2019 at 8:16 AM Thomas Stüfe wrote: > Ping... > > Guys, I need some feedback on this. If we do not fix this issue, we may > want to roll back the use of posix_spawn() as a default and return to vfork > for JDK13. > > The fix

Re: RFR(s): (new approach) 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error

2019-05-22 Thread Thomas Stüfe
Ping... Guys, I need some feedback on this. If we do not fix this issue, we may want to roll back the use of posix_spawn() as a default and return to vfork for JDK13. The fix has been tested in our nightlies for two nights in a row and did not show any errors. Cheers, Thomas On Mon, May 20,

Re: RFR(s): (new approach) 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error

2019-05-20 Thread Thomas Stüfe
Hi Florian, On Mon, May 20, 2019 at 4:24 PM Florian Weimer wrote: > * Thomas Stüfe: > > > Hi all, > > > > (old mail thread: > https://mail.openjdk.java.net/pipermail/core-libs-dev/2019-May/060139.html > ) > > > > May I please have your reviews and opinions for the following bug fix: > > > >

Re: RFR(s): (new approach) 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error

2019-05-20 Thread Florian Weimer
* Thomas Stüfe: > Hi all, > > (old mail thread: > https://mail.openjdk.java.net/pipermail/core-libs-dev/2019-May/060139.html) > > May I please have your reviews and opinions for the following bug fix: > > issue: https://bugs.openjdk.java.net/browse/JDK-8223777 > cr: >

RFR(s): (new approach) 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error

2019-05-20 Thread Thomas Stüfe
Hi all, (old mail thread: https://mail.openjdk.java.net/pipermail/core-libs-dev/2019-May/060139.html) May I please have your reviews and opinions for the following bug fix: issue: https://bugs.openjdk.java.net/browse/JDK-8223777 cr: