Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dropping DefaultGuards in wrong order can leave wrong dispatch active #1657

Open
lilyball opened this issue Oct 20, 2021 · 0 comments
Open
Labels
crate/core Related to the `tracing-core` crate kind/bug Something isn't working

Comments

@lilyball
Copy link
Contributor

Bug Report

Version

asdf-tracing v0.1.0 (/Users/lily/Dev/Scratch/asdf-tracing)
├── tracing v0.1.29
│   ├── tracing-attributes v0.1.18 (proc-macro)
│   └── tracing-core v0.1.21
└── tracing-subscriber v0.2.25
    ├── tracing v0.1.29 (*)
    ├── tracing-core v0.1.21 (*)
    ├── tracing-log v0.1.2
    │   └── tracing-core v0.1.21 (*)
    └── tracing-serde v0.1.2
        └── tracing-core v0.1.21 (*)

Platform

Darwin DeerBook 20.6.0 Darwin Kernel Version 20.6.0: Mon Aug 30 06:12:21 PDT 2021; root:xnu-7195.141.6~3/RELEASE_X86_64 x86_64

Crates

tracing-core

Description

Dropping DefaultGuards in a different order than they were created will leave the wrong dispatch active. If I make two set_default calls and drop their guards in the opposite order, the dispatch/subscriber given to the first set_default call will remain active after both guards have been dropped.

Reproduction

use std::mem;
use tracing::{info, info_span, subscriber::set_default};

fn main() {
    tracing_subscriber::fmt().with_target(false).pretty().init();
    let _compact = set_default(
        tracing_subscriber::fmt()
            .with_target(false)
            .compact()
            .finish(),
    );
    let _span = info_span!("compact").entered();
    info!("compact event");
    let _full = set_default(tracing_subscriber::fmt().with_target(false).finish());
    let _span2 = info_span!("full").entered();
    info!("full event");
    drop(_compact); // this clears the dispatch back to root
    info!("pretty event?!");
    drop(_full); // this sets it to compact
    info!("oops back to compact even though _compact was dropped");

    mem::forget((_span, _span2)); // see https://github.com/tokio-rs/tracing/issues/1656
}
Oct 19 22:43:54.441  INFO compact: compact event
Oct 19 22:43:54.442  INFO full: full event
  Oct 19 22:43:54.442  INFO pretty event?!
    at src/main.rs:18

Oct 19 22:43:54.442  INFO compact: oops back to compact even though _compact was dropped

Proposal

The default dispatch should be treated as a stack, where each DefaultGuard removes its associated dispatch from the stack. If it was not the topmost dispatch, the current default won't change, and removing the second dispatch before the topmost dispatch will properly restore the third dispatch upon removing the topmost.

One way to implement this is to make the default dispatch into a doubly-linked list, such that the guard holds a reference to the node it inserts. This way each guard can easily remove its corresponding node from the list.

@hawkw hawkw added crate/core Related to the `tracing-core` crate kind/bug Something isn't working labels Oct 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
crate/core Related to the `tracing-core` crate kind/bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants