Re: [PATCH v2] die(): stop hiding errors due to overzealous recursion guard

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 24, 2017, 18:32 UTC
Message-ID: <xmqqbmpd9qyg.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20170624123639.scagiuzqalfr72fz@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 18 quoted lines

>> One case I'd be worried about would be that the race is so bad that
>> die-is-recursing-builtin never returns 0 even once.  Everybody will
>> just say "recursing" and die, without giving any useful information.
>
> I was trying to think how that would happen. If nobody's actually
> recursing indefinitely, then the value in theory peaks at the number of
> threads (modulo the fact that we're modifying a variable from multiple
> threads without any locking; I'm not sure how reasonable it is to assume
> in practice that sheared writes may cause us to lose an increment but
> not to put nonsense in to the variable). If they are, then one thread
> may increment it to 1024 before another thread gets a chance to say
> anything. But in that case, the recursion-die is our expected outcome.
>
> Anyway, it might be reasonable to protect the counter with a mutex.
> Like:
> ...
> To be honest, I'm not sure if it's worth giving it much more time,
> though. I'd be fine with Ævar's patch as-is.

The scenario I had in mind was three or more threads simultaneously dying, each incrementing dying counter by one and before any of them have a chance to say "called many times, error or racy threaded death!", because they all observe three (or more).

But I was incorrectly reading the code---in that case, as long as dying is small enough, we'll return 0 and let at least one of the caller give a chance to give a message that came in "err" from their invocations of die()'s.

So I do not think it is worth worrying about too deeply.

Thanks.