RFR 9: 8086208 : java/lang/ProcessHandle/OnExitTest.java: IllegalThreadStateException: process hasn't exited

[Martin Buchholz]

OK, it wasn't clear to me this code was only running in a forkjoin pool.

Hmmm ... is there guidance on how tasks in a forkjoin pool should handle
InterruptedException?

It should be very rare for forkjoin threads to be externally interrupted
(task cancellation does not interrupt), so rare that maybe it should be
interpreted as an emergency request and failing with InterruptedException
is best?

I've obviously lost touch with what this modern Process code is doing ...
but:

It looks fishy that you have a future that takes an exitStatus but calls
waitFor in its body.
If the exitStatus is already available, then the process must (should?)
have already completed?

It looks fishy to call handle (which gets any exception thrown) but then
the exception is discarded.

shouldReap looks weird to me - I'd expect to always have exactly one grim
reaper thread per Process that always does the reaping.

But yeah, I already had my chance to review this code ... maybe it's time
for me to retire from java.lang.Process.


On Mon, Jun 15, 2015 at 2:21 PM, Roger Riggs <Roger.Riggs at oracle.com> wrote:

>  Hi Martin,
>
> Since the function is called from the FJ pool; what would result from
> propagating the interrupt?
> The FJPool reports the interrupted to the task but otherwise just clears
> it.
> It does not affect the completion status.
>
> Since it is known that the process has terminated and is only waiting for
> the
> Thread in ProcessImpl to post the status; it cannot/should not proceed
> until the
> exitValue is valid.
>
> The webrev has been updated to propagate the interrupted status but it
> will
> likely be ignored/discarded.
>
> Roger
>
>
>
>
> On 6/15/2015 4:46 PM, Martin Buchholz wrote:
>
> if you do get interrupted, the interrupt is swallowed, which seems wrong.
>
>  Other waiting methods have "uninterruptible" variants, that restore the
> interrupt status, like Semaphore.acquireUninterruptibly.  Should there be a
> Process.waitForUninterruptibly?
>
> On Mon, Jun 15, 2015 at 1:37 PM, Roger Riggs <Roger.Riggs at oracle.com>
> wrote:
>
>> Please review a fix for a (Unix) race condition for the exit status of
>> Process.onExit.
>> And some source cleanup of OnExitTest.
>>
>> Webrev:
>>    http://cr.openjdk.java.net/~rriggs/webrev-race-8086208/
>>
>> Issue:
>>    https://bugs.openjdk.java.net/browse/JDK-8086208
>>
>> Roger
>>
>>
>
>