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

tracing-core: Add Dispatch::downgrade() and WeakDispatch #2293

Merged
merged 9 commits into from Sep 24, 2022
18 changes: 18 additions & 0 deletions tracing-core/src/collect.rs
Expand Up @@ -79,6 +79,24 @@ use core::ptr::NonNull;
/// [`event_enabled`]: Collect::event_enabled
pub trait Collect: 'static {
/// Invoked when this collector becomes a [`Dispatch`].
///
/// ## Avoiding Memory Leaks
///
/// Collectors should not store their own [`Dispatch`]. Because the
/// `Dispatch` owns the collector, storing the `Dispatch` within the
/// collector will create a reference count cycle, preventing the `Dispatch`
/// from ever being dropped.
///
/// Instead, when it is necessary to store a cyclical reference to the
/// `Dispatch` within a collector, use [`Dispatch::downgrade`] to convert a
/// `Dispatch` into a [`WeakDispatch`]. This type is analogous to
/// [`std::sync::Weak`], and does not create a reference count cycle. A
/// [`WeakDispatch`] can be stored within a collector without causing a
/// memory leak, and can be [upgraded] into a `Dispatch` temporarily when
/// the `Dispatch` must be accessed by the collector.
///
/// [`WeakDispatch`]: crate::dispatch::WeakDispatch
/// [upgraded]: crate::dispatch::WeakDispatch::upgrade
fn on_register_dispatch(&self, collector: &Dispatch) {
hawkw marked this conversation as resolved.
Show resolved Hide resolved
let _ = collector;
}
Expand Down
115 changes: 113 additions & 2 deletions tracing-core/src/dispatch.rs
Expand Up @@ -150,11 +150,10 @@ use core::{
use std::{
cell::{Cell, RefCell, RefMut},
error,
sync::Weak,
};

#[cfg(feature = "alloc")]
use alloc::sync::Arc;
use alloc::sync::{Arc, Weak};

#[cfg(feature = "alloc")]
use core::ops::Deref;
Expand All @@ -169,6 +168,34 @@ pub struct Dispatch {
collector: &'static (dyn Collect + Send + Sync),
}

/// `WeakDispatch` is a version of [`Dispatch`] that holds a non-owning reference
/// to a [collector].
///
/// The collector may be accessed by calling [`WeakDispatch::upgrade`],
/// which returns an `Option<Dispatch>`. If all [`Dispatch`] clones that point
/// at the collector have been dropped, [`WeakDispatch::upgrade`] will return
/// `None`. Otherwise, it will return `Some(Dispatch)`.
///
/// A `WeakDispatch` may be created from a [`Dispatch`] by calling the
/// [`Dispatch::downgrade`] method. The primary use for creating a
/// [`WeakDispatch`] is to allow a collector to hold a cyclical reference to
/// itself without creating a memory leak. See [here] for details.
///
/// This type is analogous to the [`std::sync::Weak`] type, but for a
/// [`Dispatch`] rather than an [`Arc`].
///
/// [collector]: Collect
/// [`Arc`]: std::sync::Arc
/// [here]: Collect#avoiding-memory-leaks
#[derive(Clone)]
pub struct WeakDispatch {
#[cfg(feature = "alloc")]
collector: Kind<Weak<dyn Collect + Send + Sync>>,

#[cfg(not(feature = "alloc"))]
collector: &'static (dyn Collect + Send + Sync),
}

#[cfg(feature = "alloc")]
#[derive(Clone)]
enum Kind<T> {
Expand Down Expand Up @@ -577,6 +604,33 @@ impl Dispatch {
me
}

/// Creates a [`WeakDispatch`] from this `Dispatch`.
hawkw marked this conversation as resolved.
Show resolved Hide resolved
///
/// A [`WeakDispatch`] is similar to a [`Dispatch`], but it does not prevent
/// the underlying [collector] from being dropped. Instead, it only permits
/// access while other references to the collector exist. This is equivalent
/// to the standard library's [`Arc::downgrade`] method, but for `Dispatch`
/// rather than `Arc`.
///
/// The primary use for creating a [`WeakDispatch`] is to allow a collector
/// to hold a cyclical reference to itself without creating a memory leak.
/// See [here] for details.
///
/// [collector]: Collect
/// [`Arc::downgrade`]: std::sync::Arc::downgrade
/// [here]: Collect#avoiding-memory-leaks
pub fn downgrade(&self) -> WeakDispatch {
#[cfg(feature = "alloc")]
let collector = match &self.collector {
Kind::Global(dispatch) => Kind::Global(*dispatch),
Kind::Scoped(dispatch) => Kind::Scoped(Arc::downgrade(dispatch)),
};
#[cfg(not(feature = "alloc"))]
let collector = &self.collector;

WeakDispatch { collector }
}

#[cfg(feature = "std")]
pub(crate) fn registrar(&self) -> Registrar {
Registrar(match self.collector {
Expand Down Expand Up @@ -859,6 +913,63 @@ where
}
}

impl WeakDispatch {
/// Attempts to upgrade this `WeakDispatch` to a [`Dispatch`].
///
/// Returns `None` if the referenced `Dispatch` has already been dropped.
///
/// ## Examples
///
/// ```
/// # use tracing_core::collect::NoCollector;
/// # use tracing_core::dispatch::Dispatch;
/// static COLLECTOR: NoCollector = NoCollector::new();
/// let strong = Dispatch::new(COLLECTOR);
/// let weak = strong.downgrade();
///
/// // The strong here keeps it alive, so we can still access the object.
/// assert!(weak.upgrade().is_some());
///
/// drop(strong); // But not any more.
/// assert!(weak.upgrade().is_none());
/// ```
pub fn upgrade(&self) -> Option<Dispatch> {
#[cfg(feature = "alloc")]
let collector = match &self.collector {
Kind::Global(dispatch) => Some(Kind::Global(*dispatch)),
Kind::Scoped(dispatch) => dispatch.upgrade().map(Kind::Scoped),
};
#[cfg(not(feature = "alloc"))]
let collector = Some(self.collector);

collector.map(|collector| Dispatch { collector })
}
}

impl fmt::Debug for WeakDispatch {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match &self.collector {
#[cfg(feature = "alloc")]
Kind::Global(collector) => f
.debug_tuple("WeakDispatch::Global")
.field(&format_args!("{:p}", collector))
.finish(),

#[cfg(feature = "alloc")]
Kind::Scoped(collector) => f
.debug_tuple("WeakDispatch::Scoped")
.field(&format_args!("{:p}", collector))
.finish(),

#[cfg(not(feature = "alloc"))]
collector => f
.debug_tuple("WeakDispatch::Global")
.field(&format_args!("{:p}", collector))
.finish(),
}
}
}

#[cfg(feature = "std")]
impl Registrar {
pub(crate) fn upgrade(&self) -> Option<Dispatch> {
Expand Down
18 changes: 18 additions & 0 deletions tracing-subscriber/src/subscribe/mod.rs
Expand Up @@ -719,6 +719,24 @@ where
/// Performs late initialization when installing this subscriber as a
/// [collector].
///
/// ## Avoiding Memory Leaks
///
/// Subscribers should not store the [`Dispatch`] pointing to the collector
/// that they are a part of. Because the `Dispatch` owns the collector,
/// storing the `Dispatch` within the collector will create a reference
/// count cycle, preventing the `Dispatch` from ever being dropped.
///
/// Instead, when it is necessary to store a cyclical reference to the
/// `Dispatch` within a subscriber, use [`Dispatch::downgrade`] to convert a
/// `Dispatch` into a [`WeakDispatch`]. This type is analogous to
/// [`std::sync::Weak`], and does not create a reference count cycle. A
/// [`WeakDispatch`] can be stored within a subscriber without causing a
/// memory leak, and can be [upgraded] into a `Dispatch` temporarily when
/// the `Dispatch` must be accessed by the subscriber.
///
/// [`WeakDispatch`]: crate::dispatch::WeakDispatch
/// [upgraded]: crate::dispatch::WeakDispatch::upgrade
///
/// [collector]: tracing_core::Collect
fn on_register_dispatch(&self, collector: &Dispatch) {
let _ = collector;
Expand Down
2 changes: 1 addition & 1 deletion tracing/src/dispatch.rs
Expand Up @@ -148,7 +148,7 @@ pub use tracing_core::dispatch::with_default;
#[cfg_attr(docsrs, doc(cfg(feature = "std")))]
pub use tracing_core::dispatch::DefaultGuard;
pub use tracing_core::dispatch::{
get_default, set_global_default, Dispatch, SetGlobalDefaultError,
get_default, set_global_default, Dispatch, SetGlobalDefaultError, WeakDispatch,
};

/// Private API for internal use by tracing's macros.
Expand Down