subscriber: fix issues when a span triggers its parents to close

[hawkw]

This branch fixes a couple of issues which might result in child spans
not correctly triggering their parents to close:

• subscriber: fix spans closed by their children not being removed

The issue here is that the CloseGuard removes the span from the
registry (dropping the DataInner) before it decrements CLOSE_COUNT.
Dropping the DataInner will decrement the parent span's ref count,
potentially allowing it to close. When the new close for the parent span
starts, CLOSE_COUNT will be 1, not 0, and it will not be decremented
until after the try_close call completes. This means that although the
close is processed and all the layers see on_close calls for that
span, the span's data is never removed from the registry, effectively
leaking it.

This commit fixes the problem by decrementing CLOSE_COUNT before we
trigger the parent to close.

Fixes #511

• subscriber: fix child spans not closing grandparents

Currently, in situations where multiple parent spans are kept open by a
child, only the first parent will be successfully closed. This is
because calling dispatcher::get_default unsets the default dispatcher
temporarily, so that nested get_default calls will receive a no-op
subscriber. This was intended to prevent infinite dispatch loops when a
subscriber calls into code that emits its own traces. Unfortunately,
this means that if dropping the parent span would then trigger an
additional span to drop, the dispatcher is now unset.

This commit fixes this by cloning the dispatcher out of the
get_default closure before trying to close the parent span. This
means that the get_default calls are no longer nested. This does have
the disadvantage of introducing an additional Arc increment/decrement
to the span-closing path. The impact of that shouldn't be too big, but
if necessary we can try to optimize this in the future.

I've also added tests for these issues.

Signed-off-by: Eliza Weisman [email protected]

[davidbarsky]

This looks good, but I think I'd appreciate the bug descriptions above the tests as comments. It provides really rich context.