Hi Martin,
Two cases, one current and one future.
In the current case, Process can spawn a process and the process
can exit before Process can register the callback to get the exitValue.
Peter pointed out this race in his comments.
The exitValue needs to be saved (for some time yet to be determined)
to allow the Process to register and get its callback with the exitValue.
The second case is future looking to the case where a child process
not spawned by Process is exiting. It might be due to a child being
inherited from a dieing child or due to some different subprocess launcher.
When the JEP 102 process work happens, it should be possible to
wait or get called back for those non-spawned processes.
The single (smaller) number of threads has been requested to handle
processes that control a large number of children. It could be from
a thread pool, either dedicated or common. The common threadpool
does not expect its tasks to hang for an indefinite period as might
occur when waiting for along running process to exit.
Thanks, Roger
On 3/25/14 2:39 PM, Martin Buchholz wrote:
What happens if the pid you get back is a subprocess not created by
ProcessBuilder?
+ pid = waitpid(-1, &exitValue, 0);
---
What is the advantage of having a single thread? Are you just trying
to save threads? The cost of the reaper threads is much lower than
usual because of the small stack size.
On Tue, Mar 25, 2014 at 7:05 AM, Peter Levart <peter.lev...@gmail.com
<mailto:peter.lev...@gmail.com>> wrote:
On 03/24/2014 10:05 PM, roger riggs wrote:
Hi Rob, Martin, et.al <http://et.al>.
I've prototyped (for 9) a thread reaper[1] that uses a single
thread to wait for exiting
processes and calling back to the process with the exit status.
the interesting part is getting the exit status back to the
Process that needs it
It needs more testing and hardening.
I had not considered using a signal handler for SIGCHLD but
that's an option,
though we need to be very careful about thread usage.
Roger
p.s. comments on the single thread reaper appreciated (in a new
thread)
[1] http://cr.openjdk.java.net/~rriggs/webrev-waitpid/
<http://cr.openjdk.java.net/%7Erriggs/webrev-waitpid/>
Hi Roger,
I think I found a little race. Suppose reaper task is still alive
and that all consumers have been serviced (consumerCount is 0).
The reaper task waits for 500 millis for next consumer to be
registered, but times out. Before calling
"reaperThread.release()", new consumer comes around and registers
itself, also calling runReaper(), but since reaperThread.release()
has not yet been called by old reaper task, new reaper task is not
submitted to commonPool. The old reaper task finishes, leaving one
consumer on the waitingList with no reaper task to service it. If
no new consumers get registered, the waiting consumer will never
be notified...
The simplest solution for this race, I think, would be to have a
dedicated long-running thread. It could be spawned lazily, but
then it would never finish.
Otherwise a nice solution with two lists (exitList/waitList) and
avoidance of race with reversed orders between
- consumer registration: register on waitList 1st then check
exitList, and
- exit event dispatch: register on exitList 1st then check waitList
...but the check you use with consumeCount local variable to
detect processes spawned by other means (for purposes of logging
only) has a race:
thread1: Suppose a new consumer is being registered with
onExitCall(...), is added on the waitList, but before checking
exitList().size() and iterating the exitList, ...
thread2: the reaper task detects that the very same process has
finished (gets it's pid from waitpid) and adds it's pid to
exitList. Then before iterating waitList, ...
thread1: iterates the exitList, finds a match and consumes the
pid, removing the matching entries from both exitList and
waitList. Then ...
thread2: the reaper task iterates waitList, doesn't find a
matching entry for exitPid, doesn't increment consumeCount and
voila: debug log("Unexpected process exit for pid:...").
That's enough races for today.
Regards, Peter
On 3/24/2014 12:38 AM, Rob McKenna wrote:
Hi folks,
Roger Riggs (cc'd) may want to chip in here as he's looking at
the reaper thread arrangement in 9 at the moment.
On another note, I too support the merging of those files. I
didn't think there was much appetite for it at the time so I
must admit this fell down my todo list. Looking at this bug did
remind me that its something worth trying though. As per Alan's
mail, I'm going to tackle it separately if you folks don't mind.
I'll have a look at Peter's changes (thanks Peter!) as soon as I
can and see about getting them in.
-Rob
On 23/03/14 22:30, Martin Buchholz wrote:
On Sun, Mar 23, 2014 at 2:34 AM, Martin Buchholz
<marti...@google.com <mailto:marti...@google.com>> wrote:
We have also thought about whether having reaper threads is
necessary. The Unix rule is that child processes should be
waited for, and some thread needs to do that. There's no
way to wait for a set of child pids, or to specify a
"completion handler". Well, you might be able to get the
newish waitid() to do what you want, but it looks like it's
not sufficient when java is running inside a process that
might do independent subprocess creation outside of the JVM.
Actually, I take it back. With sufficient work, it looks like
you can get SIGCHLD to give you pid information in siginfo_t
si_pid, and that can be used to trigger the reaping. It looks
like waitpid is "async-signal-safe", so we can call it from our
signal handler.
While we're at it we can fix SIGCHLD handling to do signal
chaining, as with other signals.
