[Patch] Rewrite PosixProcess

David Daney ddaney@avtrex.com
Fri May 11 18:05:00 GMT 2007


Andrew Haley wrote:
> David Daney writes:
>  > The bits in PosixProcess were getting stale and I thought it was time to 
>  > freshen them up a some.
>  > 
>  > The main motivation for the patch is to make it possible for libgcj to 
>  > interact with other code that forks processes and catches SIGCHLD.
>  > 
>  > The main change is that we iterate through each known PosixProcess and 
>  > try to wait specifically for it instead of all processes whenever we 
>  > receive a SIGCHLD.  This allows other code to wait for its own processes.
>  > 
>  > Tested on x86_64-pc-linux-gnu (FC6) with no failures in libjava.
>  > 
>  > OK to commit?
>
> This all seems very sensible.
>
> A nit -- why make this change:
>
>  > @@ -342,7 +325,7 @@ final class PosixProcess extends Process
>  >  	// There is no race with reap() in the pidToProcess map
>  >  	// because this is always called from the same thread
>  >  	// doing the reaping.
>  > -	pm.addProcessToMap(this);
>  > +	pm.liveProcesses.add(this);
>  >  	state = STATE_RUNNING;
>  >  	// Notify anybody waiting on state change.
>  >  	this.notifyAll();
>
> rather than changing addProcessToMap to DTRT?  Seems like you've made
> the encapsulation of ProcessManager worse for no good reason.
>
>   
It was an inadvertent encapsulation breaking micro optimization.

Here is a new version of the patch that corrects this issue.

Tested as before with no failures.

OK to commit?

2007-05-11  David Daney  <ddaney@avtrex.com>

    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.


-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: process.diff.txt
URL: <http://gcc.gnu.org/pipermail/java-patches/attachments/20070511/45a4857d/attachment.txt>


More information about the Java-patches mailing list