Currently libgcj assumes it can waitpid(-1,...). This interacts poorly with other libraries which may want to interact with subprocesses; for instance glib has an API for spawning and waiting for children, and frysk has to work around this problem (by disallowing the use of Process). libgcj ought to have a way to hook into the low-level process code so that it can interact gracefully with other libraries. I think it is ok if this is CNI only.
Hmnm, is this really a bug, waitpid is used only in the reaper thread.
Yes, it really is a bug. libgcj can reap a child process started by some other library. This means it is hard to use libgcj in conjunction with other libraries which may want to do their own subprocess bookkeeping.
One way to fix it would be to have a reaper thread for *each* Process. Then the reaper could do a waitpid(pid...) instead of waitpid(-1...). If one only spawns a few processes, this would be fine. This would allow us to get rid of the SIGCHLD handler as well. Another option would to have the process reaper be a seperate process, but that scares me.
Subject: Re: add wait handling hook On Thu, 2006-10-05 at 05:42 +0000, daney at gcc dot gnu dot org wrote: > > ------- Comment #3 from daney at gcc dot gnu dot org 2006-10-05 05:42 ------- > One way to fix it would be to have a reaper thread for *each* Process. Then > the reaper could do a waitpid(pid...) instead of waitpid(-1...). If one only > spawns a few processes, this would be fine. This would allow us to get rid of > the SIGCHLD handler as well. > > Another option would to have the process reaper be a seperate process, but that > scares me. Even multiple threads scares me for Linux since that would mean double the amount of processes in the process table. Maybe Linux needs this problem if it still exists. I know Linux threads had this problem but I forget if NTPL does. Thanks, Andrew Pinski
Another idea: In the SIGCHLD signal handler record the pid of the process that exited. Then look it up in the pidToProcess map. If it belongs to the libgcj runtime, then do waitpid(pid, ...) on it. Otherwise ignore it assuming it belongs to code external to libgcj. It would be nice if we could call into libgcj from the signal handler, but I don't think that we can. :( You could get even fancier and chain the SIGCHLD signal handler to any handler that may have been previously installed. We would expect the same of external code that installed a SIGCHLD handler.
Subject: Bug 29324 Author: daney Date: Sat May 12 17:37:55 2007 New Revision: 124638 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=124638 Log: PR libgcj/29324 * include/posix-threads.h (_Jv_BlockSigchld): Declare. (_Jv_UnBlockSigchld): Same. * posix-threads.cc: Include posix-threads.h. (block_sigchld) Rename to... (_Jv_BlockSigchld) ... this. (_Jv_UnBlockSigchld): New function. (_Jv_InitThreads): Call _Jv_BlockSigchld in place of block_sigchld. (_Jv_ThreadStart): Same. * java/lang/PosixProcess$ProcessManager.h: Regenerate. * java/lang/PosixProcess.java: Clean up imports. (ProcessManager): Make final. (ProcessManager.queue): Genericise and make private. (ProcessManager.pidToProcess): Remove. (ProcessManager.liveProcesses): New field. (ProcessManager.reaperPID): Remove. (ProcessManager.nativeData): New field. (ProcessManager.removeProcessFromMap): Remove. (ProcessManager.addProcessToMap):Remove. (ProcessManager.addToLiveProcesses): New method. (ProcessManager.run): Rewritten. (ProcessManager.reap): Change method signature, (getErrorStream): Correct formatting. (getInputStream): Same. (spawn): Add process to liveProcesses list. (pid): Make package private. * java/lang/PosixProcess.h: Regenerate. * java/lang/natPosixProcess.cc: Include posix.h and posix-threads.h. Add useing namespace java::lang. (ProcessManagerInternal): New struct. (sigchld_handler): Rewritten. (init): Rewritten. (waitForSignal): Same. (reap): Same. (signalReaper): Same. (nativeDestroy): Call kill as ::kill. (nativeSpawn): Correct formatting. * classpath/lib/java/lang/PosixProcess$EOFInputStream.class: Regenerate. * classpath/lib/java/lang/PosixProcess.class: Same. * classpath/lib/java/lang/PosixProcess$ProcessManager.class: Same. Modified: trunk/libjava/ChangeLog trunk/libjava/classpath/lib/java/lang/PosixProcess$EOFInputStream.class trunk/libjava/classpath/lib/java/lang/PosixProcess$ProcessManager.class trunk/libjava/classpath/lib/java/lang/PosixProcess.class trunk/libjava/include/posix-threads.h trunk/libjava/java/lang/PosixProcess$ProcessManager.h trunk/libjava/java/lang/PosixProcess.h trunk/libjava/java/lang/PosixProcess.java trunk/libjava/java/lang/natPosixProcess.cc trunk/libjava/posix-threads.cc
I think the patch solves the problem.