RE: [RFC] GC: external thread suspension

["Boehm, Hans" <hans dot boehm at hp dot com>]

Thanks.

In general, this looks pretty good to me.  As you no doubt discovered,
this is tricky code.

I copied Paolo Malaro, and reattached Keith's patch for his benefit,
since I think Mono takes a vaguely analogous approach to stopping
threads for debugging.  It would be useful to make sure that we are
converging instead of diverging further.

Here are a few more comments:

Signal handler:

UNLOCK() is generally not asynch-signal-safe.  This needs a comment
stating that this only happens if it's directly called.  Is it better to
suspend oneself using something like the GC_start_blocking() support?
(Note that this is fixed and renamed GC_do_blocking() in 7.0, and maybe
hard to use in 6.7.)

Redundant parens in while loop at end.  Shouldn't one of the && be an
||?  This doesn't look quite right.

What happens if a thread is suspended in the final loop, and multiple
GCs occur? Can it acknowledge the suspension?

suspend_thread:
If externally suspended, can it wake up asynchronously?
Add heading comment and/or rename, since this doesn't really suspend a
thread;
it only suspends the signal.

GC_stop_world_inner:
Rename to GC_stop_threads?  I don't like the current name, since that's
no longer always what it does.

I don't like the resume_thread vs GC_resume_thread naming.  How about
send_suspend_signal and send_resume_signal?

I'm aiming for #if's which are properly indented, but with the # in
column 1.
In gc6.7, that isn't always the case anyway, but I fixed it in 7.0, and
would
like to see new code follow that convention.

Hans



> -----Original Message-----
> From: java-patches-owner@gcc.gnu.org 
> [mailto:java-patches-owner@gcc.gnu.org] On Behalf Of Keith Seitz
> Sent: Thursday, May 11, 2006 6:11 PM
> To: Bryce McKinlay
> Cc: Java Patch List
> Subject: Re: [RFC] GC: external thread suspension
> 
> Bryce McKinlay wrote:
> 
> > We will need a ChangeLog entry for this patch.  Also, it looks like 
> > changes to gc.h to add the public interface weren't included?
> 
> Yeah, that's just an RFC for comment. I won't actually post a 
> changelog until discussions are complete and we have a final 
> version which can be posted.
> 
> You are correct: no public declaration in gc.h. I forgot to 
> do that! My bad.
> 
> Keith
>

Attachment: gc-suspend.patch
Description: gc-suspend.patch