Re: Patch for TearDownProcess

[Kris Van Hees <kris dot van dot hees at oracle dot com>]

On Fri, Sep 07, 2007 at 09:30:28AM -0400, Andrew Cagney wrote:
> Kris Van Hees wrote:
>> The attached patch *does* open the possibility that a stray process may 
>> remain
>> after tearDown() has been executed if e.g. a process termination causes 
>> another
>> process to be created, etc...  However, that will not cause a testsuite 
>> hang.
>> It is also not expected to pose a problem with the further execution of 
>> the
>> testsuite (in fact, it is very likely to get cleaned up during the 
>> tearDown()
>> of the next test).
>>   
> Unfortunately this isn't theoretical.  On a slower f5 machine; this happens 
> consistently; invalidating test results.

The information you added to ticket 4996 doesn't quite show anything that
indicates that this is a problem with my patch.  In the future, could you
at least add output after a -c FINEST run or something, to ensure there is
relevant information there to help work out a fix?

> Given the choices between a potential test-suite hang, and tear-down 
> leaving waitpid events around, the decision was made in favor of the 
> latter.  That decision hasn't changed.  Having the test run hang, is a 
> lesser evil then the test-suite continuing but producing bogus results.. 
> I restored the old behavior; and then added a timeout.  It currently logs a 
> message, I suspect it should abandon the test run, since the problem state 
> hasn't gone away.

Obviously, there can be a difference of opinion on this matter, and I believe
that reversing this patch without any consideration for the matter it aims at
solving is useless.  You deliberately restore a problem behaviour.  Why?

It would have been more constructive to open a bug about the problem you have
encountered, and have that problem resolved, so that in the end we have a
fully working testsuite that doesn't need a tradeoff between hanging and
stray waitpid events.

Finally, given that you use FC5 as your reference platform here, perhaps you
could add it to the automated test system (i.e. have it submit results there)
so that test coverage can be expended to that release as well (especially since
it seems to uncover some problems that other systems do not - assuming of
course that no kernel issues are involved).

	Cheers,
	Kris