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

[Thomas Stüfe]

On Tue, Jun 4, 2019 at 7:21 PM Martin Buchholz <martinrb at 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.stuefe at 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.stuefe at 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
>>>
>>>
>>>