Re: [PATCH v2 0/2] avoid bogus "recursion detected in die handler" message

From: Jeff King <peff@peff.net>
Date: Apr 17, 2013, 01:37 UTC
Message-ID: <20130417013708.GA27786@sigill.intra.peff.net>
In-Reply-To: <20130417004941.GJ29773@google.com>

On Tue, Apr 16, 2013 at 05:49:41PM -0700, Jonathan Nieder wrote:

Show 11 quoted lines

> Jeff King wrote:
> 
> >   [1/2]: usage: allow pluggable die-recursion checks
> >   [2/2]: run-command: use thread-aware die_is_recursing routine
> 
> Lovely.  This doesn't solve the analagous problem for grep,
> index-pack, pack-objects, preload-index, or bidirectional_transfer_loop,
> but it doesn't make them worse and even should make them easier to fix
> later, so
> 
> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>

The problem does exist in those other programs, but is much less likely to be triggered. There it would be a race between two threads calling die() at the same time, because as soon as one finishes, it takes down the whole program. What made this one so easy to trigger was the installation of die_async.

So it's probably not that big a deal to leave the race in those other places for the time being. Any refactoring of the "dying" flag should accompany an overall strategy for managing the clean death of sub-threads (which would entail a custom die routine, at which point the recursion-check should just fall out naturally from whatever approach they choose).

Thanks for reviewing.

-Peff