[
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.
>