[Gc] Re[4]: [bdwgc] Exit abort hooks

Ivan Maidanski ivmai at mail.ru
Sat Jun 30 08:50:57 PDT 2012


Hi Jean-Claude,

Mon, 25 Jun 2012 23:35:11 -0400 Jean-Claude Beaudoin <jean.claude.beaudoin at gmail.com>:
> Hello Ivan and Hans,
> 
> On Mon, Jun 25, 2012 at 4:00 AM, Ivan Maidanski <ivmai at mail.ru> wrote:
> 
> > I've already pushed own variant of such functionality into
> > https://github.com/ivmai/bdwgc/tree/add-abort-exit-signals-setters
> > and consists of 2 commits:
> > -
> > https://github.com/ivmai/bdwgc/commit/215d710e39b5f9df733525b97ff8c39cb659c7b0(call GC_on_abort(NULL) on exit(1));
> > -
> > https://github.com/ivmai/bdwgc/commit/d020903787f3aa4f949bd5c48083cfd71d36df6f(Add public GC_set/get_abort_func to replace default GC_on_abort)
> >
> >
> >
> After having given it a good look, all this seems to pretty well fit what I
> needed, thanks.
> 
> Now waiting for the "signals configuration at init time" thing...

Rejected. Looks to me incorrect - it seems you just add some variables and setters for them but the variables are not used in GC (e.g., resume signal is always SIG_THR_RESTART).

Let's start from the beginning: you want some new GC functionality allowing to suggest the collector to use some specific signals for POSIX threads suspension and/or resuming. To make this, I expect from you several easy-to-review commits:

    pthread_stop_world.c code refactoring replacing SIG_THR_RESTART macro usage with GC_sig_thr_restart STATIC variable (i.e. GC functionality remains total unchanged - you just define some variable, set its value to SIG_THR_RESTART and use that variable instead of SIG_THR_RESTART down the file (except for ifdef, of course);
    Add GC_set_thr_restart_signal declaration/definitions to gc.h, pthread_stop_world.c, misc.c (in the last case it should be no-op as has no effect on non-POSIX systems);
    ... (the rest 2 commits are the similar but relate to suspend signal)

When you finish the 1st patch preparation, I'll review it (and the accompanying commit log message) and if it is ok, I'll cherry-pick it to new add-thr-signals-setters branch and inform you that you could send me the second one.

Sorry for such a long procedure but sometimes it's better to divide work into small pieces to simplify verification.

PS. Of course, I could do the whole work myself but I save some part of my time by just explaining what's wrong with the patch and how should the patch look like to be accepted.

> 
> Regards,
> 
> Jean-Claude Beaudoin
> 
> 

Regards,
Ivan



More information about the Gc mailing list