On Tue, Jun 4, 2019 at 7:21 PM Martin Buchholz <marti...@google.com> 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 comments becoming stale. Can > we come up with a better name for "fail" that reflects the new > implementation? I suggest "can_johnny_exec". > > As in the can_johnny_exec_pipe and its CAN_JOHNNY_EXEC_PIPE_FD ? Sure, why not :). I agree on the complexity remark. I do not really like it either. Maybe we can remove this fix again if in the far future all posix_spawn() variants in the wild report exec errors back like they should. I believe Florian added already a patch to the glibc? Thanks, Thomas > On Mon, Jun 3, 2019 at 11:07 PM Thomas Stüfe <thomas.stu...@gmail.com> > wrote: > >> 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 <thomas.stu...@gmail.com> >> wrote: >> >>> 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: >>> http://cr.openjdk.java.net/~stuefe/webrevs/8223777-posix_spawn-no-exec-error-alternate-impl/webrev.00/webrev/ >>> >>> --- >>> >>> The fix implements Florians proposal: the jspawnhelper will signal its >>> aliveness to the parent process the moment it gains control. If the parent >>> process does not get the signal, it assumes exec'ing the jspawnhelper never >>> worked. >>> >>> I only do this for posix_spawn mode, out of cautiousness. >>> >>> (Note that I kept the fix as minimal as possible. I found a minor bug >>> and some improvement possibilities and opened follow up issues to track >>> them: JDK-8224180 and JDK-8224181). >>> >>> Thanks, Thomas >>> >>> >>>
