Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> writes:
> So let's just set the recursion limit to a number higher than the
> number of threads we're ever likely to spawn. Now we won't lose
> errors, and if we have a recursing die handler we'll still die within
> microseconds.
>
> There are race conditions in this code itself, in particular the
> "dying" variable is not thread mutexed, so we e.g. won't be dying at
> exactly 1024, or for that matter even be able to accurately test
> "dying == 2", see the cases where we print out more than one "W"
> above.
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.
Will queue, as it is nevertheless an improvement over the current
code.
Thanks.
> usage.c | 18 +++++++++++++++++-
> 1 file changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/usage.c b/usage.c
> index 2f87ca69a8..1ea7df9a20 100644
> --- a/usage.c
> +++ b/usage.c
> @@ -44,7 +44,23 @@ static void warn_builtin(const char *warn, va_list params)
> static int die_is_recursing_builtin(void)
> {
> static int dying;
> - return dying++;
> + /*
> + * Just an arbitrary number X where "a < x < b" where "a" is
> + * "maximum number of pthreads we'll ever plausibly spawn" and
> + * "b" is "something less than Inf", since the point is to
> + * prevent infinite recursion.
> + */
> + static const int recursion_limit = 1024;
> +
> + dying++;
> + if (dying > recursion_limit) {
> + return 1;
> + } else if (dying == 2) {
> + warning("die() called many times. Recursion error or racy threaded death!");
> + return 0;
> + } else {
> + return 0;
> + }
> }
>
> /* If we are in a dlopen()ed .so write to a global variable would segfault
