Re: RFA PATCH: Fix PosixProcess some more...

[Bryce McKinlay <mckinlay at redhat dot com>]

Hi David,

Nice catch. The GC uses signals internally to suspend other threads 
while the GC is running (see boehm-gc/pthread_stop_world.c).  The 
symptom you describe would happen if one thread did not enter its signal 
handler because that signal was blocked at the time the GC sent it. I 
guess this happens because sigwait() blocks any signals that arn't in 
the set you pass to it, and/or doesn't run their regular handlers.

The change is fine, but shouldn't waitForSignal() call sigsuspend() in a 
loop, and only return if the signal it got is a SIGCHLD? Otherwise, it 
seems that the GC's (or any other) signals will also wake it up. I guess 
the run() loop will just call waitForSignal() again, but it might be a 
bit inefficient if signals are being frequently delivered for some 
reason. Also, it would be good to add a comment in waitForSignal() 
noting the interaction with the GC and why sigsuspend() is neccessary.

I'm not sure the test case is acceptable if it takes 30s to run, 
however. We want the test case to be as fast and convenient to run as 
possible for GCC developers, and adding such a long fixed delay is 
probably not good.

Regards

Bryce


David Daney wrote:

> My recent changes to PosixProcess cause it to fail when GC happens while
> a Process is executing.
>
> On a glibc-2.2.5/linuxthreads system the symptoms are that all threads
> are frozen in GC until the Process terminates.
>
> On my Fedora Core 1 NPTL system, GC seems to work, but the process
> reaper thread gets killed causing other types of bad behavior.
>
> The attach patch seems to fix the problmes.
>
> Tested with make check in libjava with no regressions on:
>
> i686-pc-linux-gnu (Fedora Core 1 NPTL) and mipsel-linux-gnu (glibc
> 2.2.5) with no regressions.
>
> OK to commit?
>
> David Daney
>  
>
> ------------------------------------------------------------------------
>
> 2004-08-17  David Daney  <ddaney@avtrex.com>
>
> 	* java/lang/natPosixProcess.cc (waitForSignal): Use sigsuspend
> 	instead of sigwait.
>