Dropping dispatch::DefaultGuards out of orders leads to unintuitive Dispatch being used

[mladedav]

Bug Report

Version

Current master (908cc43), probably all prior versions since addition of set_default.

Crates

tracing-core

Description

When a local default Dispatch is set, a guard is created which restores the previous Dispatch on drop. However, this does not take into account the possibility that the dropped guard may not be the last guard or that the Dispatch that is being restored may have had its guard already dropped.

For example if one calls set_default three times with guards a, b, and c, they should be using the dispatch guarded by c. If b is dropped now, the default Dispatch is restored back to the one behind the a guard.

Example code reproduction

    fn dispatch_nesting() {
        struct TestCollector<const ID: u64>;
        impl<const ID: u64> Collect for TestCollector<ID> {
            fn enabled(&self, _: &Metadata<'_>) -> bool {
                true
            }

            fn new_span(&self, _: &span::Attributes<'_>) -> span::Id {
                span::Id::from_u64(ID)
            }

            fn record(&self, _: &span::Id, _: &span::Record<'_>) {}

            fn record_follows_from(&self, _: &span::Id, _: &span::Id) {}

            fn event(&self, _: &Event<'_>) {}

            fn enter(&self, _: &span::Id) {}

            fn exit(&self, _: &span::Id) {}

            fn current_span(&self) -> span::Current {
                span::Current::unknown()
            }
        }

        let guard_a = set_default(&Dispatch::new(TestCollector::<0xAAAA>));
        let default_dispatcher = Dispatch::default();
        assert!(default_dispatcher.is::<TestCollector::<0xAAAA>>());

        let guard_b = set_default(&Dispatch::new(TestCollector::<0xBBBB>));
        let default_dispatcher = Dispatch::default();
        assert!(default_dispatcher.is::<TestCollector::<0xBBBB>>());

        let guard_c = set_default(&Dispatch::new(TestCollector::<0xCCCC>));
        let default_dispatcher = Dispatch::default();
        assert!(default_dispatcher.is::<TestCollector::<0xCCCC>>());

        drop(guard_b);
        let default_dispatcher = Dispatch::default();
        assert!(default_dispatcher.is::<TestCollector::<0xAAAA>>());

        drop(guard_c);
        let default_dispatcher = Dispatch::default();
        assert!(default_dispatcher.is::<TestCollector::<0xBBBB>>());

        drop(guard_a);
        let default_dispatcher = Dispatch::default();
        assert!(default_dispatcher.is::<NoCollector>());
    }

Proposal

I think that the latest Dispatch for which a guard was not dropped should be used. Tracking of the state will become more complex.

Alternatively, this could be ignored as it seems that no one has encountered this bug so far and the usage of set_default like this seems rather contrived.