Hi Frank
Thanks for the review.
On 9/6/2017 1:41 AM, Frank Yuan wrote:
Hi Roger
I think this is the cause and I believe the patch will work.
However, I have the following concern and minor comments:
a. Is it possible that following scenario happens?
1. process A exits
2. Completion thread in onExist runs and gets 0 as origStart, and then
sleeps 5s
3. process reaper thread reaps the process A, and then the pid is reused
4. Completion thread gets a value as startTime, and then has to wait the
new process termination
I am not sure if the startTime may be zero except the process is a zombie, if the
startTime may be zero only when the process is a zombie, we can change the condition to:
if (startTime > 0 && startTime != origStart), otherwise we can't avoid such
scenario, but if ProcessHandleImpl.onExit() passes startTime field to function completion,
it may be a little bit better, at least, onExit() can be consistent with isAlive().
Note that the startTime in the ProcessHandle is acquired by
ProcessHandle.of which uses isAlive,
and may be 0.
StartTime is optional and the provision to ignore startTime = 0 is
intended to cover cases
where the underlying operating system did not or could not provide a
reasonable start time.
We have not see any issues with pid re-use and I would prefer to avoid
over engineering the code
for a non-issue.
With the proposed fix, the onExit completion handler and isAlive are
consistent.
b. Should we rollback the change of JDK-8184808, at least, update the comment
in that change?
I'll update the comment. Double checking using kill seems to be more
reliable.
c. Should we remove @key intermittent and the debug info added in JDK-8183019?
I'd prefer to leave them for some to make sure the issue does not re-appear.
I created a subtask to re-check in 2 months.
Roger
Thanks
Frank
-----Original Message-----
From: core-libs-dev [mailto:[email protected]] On Behalf
Of Roger Riggs
Subject: RFR 8177932 (process) java/lang/ProcessHandle/OnExitTest.java failed with
"Process A should not be alive"
Please review a fix for an intermittent issue with ProcessHandle.onExit.
On Solaris, the start time of a process reported through
/proc/pid/psinfo changes to
zero when the process is exiting. The onExit implementation incorrectly
interpreted zero
meaning the pid had been re-used and the process was no longer alive.
However,
ProcessHandle.isAlive considered zero to be missing information and the
process was still alive.
Webrev:
http://cr.openjdk.java.net/~rriggs/webrev-onexit-8177932/
Issue:
https://bugs.openjdk.java.net/browse/JDK-8177932
Thanks, Roger
