Skip to main content

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [cdt-patch] Patches to process management libraries and classes

> 

Sorry for the delay.

> The third patch file, org.eclipse.cdt.core.patch, addresses a problem I =
> was seeing with Spawner/Reaper synchronization in my environment.  The =

Can you tell me more about the problem.  I did not notice anything so
far, tested on GNU/Linux.

> command would run, produce data on stdout, but the Reaper's output =
> buffer was empty.

Reaper's buffer?  The Reaper's thread goal is to do a waitpid()
to get rid of any zombies.  And GNU/Linux it is even more peculiar
because of the threading(clone()) where the signal is only deliver
to a particular thread.  Not sure, I follow about the Reaper's buffer,
do you mean the process buffer?

> I addressed this problem by separating the Reaper =
> class from the Spawner class and changed the constructor to make the =
> explicit the parent/child relationship between these classes.  So  =
> instead of using the Spawner.this as a synchronization object, the =
> Reaper's constructor accepts a reference its parent object and uses that =
> reference for synchronization.

I see 0 advantage in this, the only change is coding style, wheter to
pass the object as argument to a constructor or use the the reference Spawner.this
is, at least to me, equivalent.

> I also create the in/out/err streams =
> before releasing the lock on the object and the accessors also lock the =
> object before returning the references to the in/out/err streams.
> 

I do not see what is gain here.
	public InputStream getInputStream() {
		synchronized (this) {
			return in;
		}
	}
The in/out/err streams are created in the constructor, why the need 
to synchronized? No race or critical sections.
The exec() method is always call in the constructor.


> I would appreciate if the maintainer of this code could review my =
> changes incorporate them into the project's repository.
> 

Gene, I really do not see the race conditions or what the patch
is trying to fix.  Some part of the patch was coding style 8-)
which is allright by me, if its make things clearer.

> 



Back to the top