tracing-core: Add Dispatch::downgrade() and WeakDispatch
#2293
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jswrenn
requested review from
hawkw,
carllerche,
davidbarsky and
a team
as code owners
|
@jswrenn if this won't get merged soon, let's point the PR (or make a separate one) for the |
|
why does this need to be pointed at the |
hawkw
reviewed
Sep 7, 2022
|
@jswrenn any updates on this? |
Allows collectors and subscribers to stash their own `Dispatch` without causing a memory leak.
jswrenn
force-pushed
the
dispatch-cycle-breaking
branch
from
d50d6f8
to
03786ca
Compare
hawkw
requested changes
Sep 20, 2022
|
I'd like to get this one merged before cutting another |
|
Thanks! I agree with all your suggestions! |
hawkw
approved these changes
Sep 24, 2022
This was creating an `&&'static (dyn Collect + ...)` rather than copying the `&'static (dyn Collect + ...)` into the `WeakDispatch` struct, and the reference to a static reference to a `dyn Collect + ...` doesn't forward trait impls. Removing the `&` un-breaks this.
hawkw
added a commit
that referenced
this pull request
Sep 24, 2022
Allows `Subscriber`s and `Layer`s to stash their own `Dispatch` without causing a memory leak. ## Motivation Resolves a shortcoming of #2269: that it's impossible for `Subscriber`s or `Layer`s to stash a copy of their own `Dispatch` without creating a reference cycle that would prevent them from being dropped. ## Solution Introduces `WeakDispatch` (analogous to `std::sync::Weak`) that holds a weak pointer to a `Subscriber`. `WeakDispatch` can be created via `Dispatch::downgrade`, and can be transformed into a `Dispatch` via `WeakDispatch::upgrade`. Co-authored-by: Eliza Weisman <eliza@buoyant.io>
hawkw
added a commit
that referenced
this pull request
Sep 24, 2022
Allows `Subscriber`s and `Layer`s to stash their own `Dispatch` without causing a memory leak. ## Motivation Resolves a shortcoming of #2269: that it's impossible for `Subscriber`s or `Layer`s to stash a copy of their own `Dispatch` without creating a reference cycle that would prevent them from being dropped. ## Solution Introduces `WeakDispatch` (analogous to `std::sync::Weak`) that holds a weak pointer to a `Subscriber`. `WeakDispatch` can be created via `Dispatch::downgrade`, and can be transformed into a `Dispatch` via `WeakDispatch::upgrade`. Co-authored-by: Eliza Weisman <eliza@buoyant.io>
hawkw
added a commit
that referenced
this pull request
Sep 30, 2022
Allows `Subscriber`s and `Layer`s to stash their own `Dispatch` without causing a memory leak. ## Motivation Resolves a shortcoming of #2269: that it's impossible for `Subscriber`s or `Layer`s to stash a copy of their own `Dispatch` without creating a reference cycle that would prevent them from being dropped. ## Solution Introduces `WeakDispatch` (analogous to `std::sync::Weak`) that holds a weak pointer to a `Subscriber`. `WeakDispatch` can be created via `Dispatch::downgrade`, and can be transformed into a `Dispatch` via `WeakDispatch::upgrade`. Co-authored-by: Eliza Weisman <eliza@buoyant.io>
hawkw
added a commit
that referenced
this pull request
Sep 30, 2022
Allows `Subscriber`s and `Layer`s to stash their own `Dispatch` without causing a memory leak. ## Motivation Resolves a shortcoming of #2269: that it's impossible for `Subscriber`s or `Layer`s to stash a copy of their own `Dispatch` without creating a reference cycle that would prevent them from being dropped. ## Solution Introduces `WeakDispatch` (analogous to `std::sync::Weak`) that holds a weak pointer to a `Subscriber`. `WeakDispatch` can be created via `Dispatch::downgrade`, and can be transformed into a `Dispatch` via `WeakDispatch::upgrade`. Co-authored-by: Eliza Weisman <eliza@buoyant.io>
hawkw
added a commit
that referenced
this pull request
Oct 6, 2022
# 0.1.30 (October 6, 2022) This release of `tracing-core` adds a new `on_register_dispatch` method to the `Subscriber` trait to allow the `Subscriber` to perform initialization after being registered as a `Dispatch`, and a `WeakDispatch` type to allow a `Subscriber` to store its own `Dispatch` without creating reference count cycles. ### Added - `Subscriber::on_register_dispatch` method ([#2269]) - `WeakDispatch` type and `Dispatch::downgrade()` function ([#2293]) Thanks to @jswrenn for contributing to this release! [#2269]: #2269 [#2293]: #2293
hawkw
added a commit
that referenced
this pull request
Oct 6, 2022
# 0.1.30 (October 6, 2022) This release of `tracing-core` adds a new `on_register_dispatch` method to the `Subscriber` trait to allow the `Subscriber` to perform initialization after being registered as a `Dispatch`, and a `WeakDispatch` type to allow a `Subscriber` to store its own `Dispatch` without creating reference count cycles. ### Added - `Subscriber::on_register_dispatch` method ([#2269]) - `WeakDispatch` type and `Dispatch::downgrade()` function ([#2293]) Thanks to @jswrenn for contributing to this release! [#2269]: #2269 [#2293]: #2293
hawkw
added a commit
that referenced
this pull request
Oct 6, 2022
# 0.1.37 (October 6, 2022) This release of `tracing` incorporates changes from `tracing-core` [v0.1.30][core-0.1.30] and `tracing-attributes` [v0.1.23][0.1.23], including the new `Subscriber::on_register_dispatch` method for performing late initialization after a `Subscriber` is registered as a `Dispatch`, and bugfixes for the `#[instrument]` attribute. Additionally, it fixes instances of the `bare_trait_objects` lint, which is now a warning on `tracing`'s MSRV and will become an error in the next edition. ### Fixed - **attributes**: Incorrect handling of inner attributes in `#[instrument]`ed functions (#2307) - **attributes**: Incorrect location of compiler diagnostic spans generated for type errors in `#[instrument]`ed `async fn`s (#2270) - **attributes**: Updated `syn` dependency to fix compilation with `-Z minimal-versions` (#2246) - `bare_trait_objects` warning in `valueset!` macro expansion (#2308) ### Added - **core**: `Subscriber::on_register_dispatch` method (#2269) - **core**: `WeakDispatch` type and `Dispatch::downgrade()` function (#2293) ### Changed - `tracing-core`: updated to [0.1.30][core-0.1.30] - `tracing-attributes`: updated to [0.1.23][attrs-0.1.23] ### Documented - Added [`tracing-web`] and [`reqwest-tracing`] to related crates (#2283], [#2331) Thanks to new contributors @compiler-errors, @e-nomem, @WorldSEnder, @Xiami2012, and @tl-rodrigo-gryzinski, as well as @jswrenn and @CAD97, for contributing to this release! [core-0.1.30]: https://github.com/tokio-rs/tracing/releases/tag/tracing-core-0.1.30 [attrs-0.1.23]: https://github.com/tokio-rs/tracing/releases/tag/tracing-attributes-0.1.23 [`tracing-web`]: https://crates.io/crates/tracing-web/ [`reqwest-tracing`]: https://crates.io/crates/reqwest-tracing/
hawkw
added a commit
that referenced
this pull request
Oct 6, 2022
# 0.1.37 (October 6, 2022) This release of `tracing` incorporates changes from `tracing-core` [v0.1.30][core-0.1.30] and `tracing-attributes` [v0.1.23][attrs-0.1.23], including the new `Subscriber::on_register_dispatch` method for performing late initialization after a `Subscriber` is registered as a `Dispatch`, and bugfixes for the `#[instrument]` attribute. Additionally, it fixes instances of the `bare_trait_objects` lint, which is now a warning on `tracing`'s MSRV and will become an error in the next edition. ### Fixed - **attributes**: Incorrect handling of inner attributes in `#[instrument]`ed functions (#2307) - **attributes**: Incorrect location of compiler diagnostic spans generated for type errors in `#[instrument]`ed `async fn`s (#2270) - **attributes**: Updated `syn` dependency to fix compilation with `-Z minimal-versions` (#2246) - `bare_trait_objects` warning in `valueset!` macro expansion (#2308) ### Added - **core**: `Subscriber::on_register_dispatch` method (#2269) - **core**: `WeakDispatch` type and `Dispatch::downgrade()` function (#2293) ### Changed - `tracing-core`: updated to [0.1.30][core-0.1.30] - `tracing-attributes`: updated to [0.1.23][attrs-0.1.23] ### Documented - Added [`tracing-web`] and [`reqwest-tracing`] to related crates (#2283, #2331) Thanks to new contributors @compiler-errors, @e-nomem, @WorldSEnder, @Xiami2012, and @tl-rodrigo-gryzinski, as well as @jswrenn and @CAD97, for contributing to this release! [core-0.1.30]: https://github.com/tokio-rs/tracing/releases/tag/tracing-core-0.1.30 [attrs-0.1.23]: https://github.com/tokio-rs/tracing/releases/tag/tracing-attributes-0.1.23 [`tracing-web`]: https://crates.io/crates/tracing-web/ [`reqwest-tracing`]: https://crates.io/crates/reqwest-tracing/
hawkw
added a commit
that referenced
this pull request
Oct 6, 2022
)" This reverts commit a0824d3 (PR #2247). As discussed in [this comment][1], the implementation for `Arc`s may cause subtly incorrect behavior if actually used, due to the `&mut self` receiver of the `LookupSpan::register_filter` method, since the `Arc` cannot be mutably borrowed if any clones of it exist. The APIs added in PRs #2269 and #2293 offer an alternative solution to the same problems this change was intended to solve, and --- since this change hasn't been published yet --- it can safely be reverted. [1]: https://giethub.com/tokio-rs/tracing/pull/2247#issuecomment-1199924876
|
how to use |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Allows collectors and subscribers to stash their own
Dispatchwithout causing a memory leak.Motivation
Resolves a shortcoming of #2269: that it's impossible for collectors or subscribers to stash a copy of their own
Dispatchwithout creating a reference cycle that would prevent them from being dropped.Solution
Introduces
WeakDispatch(analogous tostd::sync::Weak) that holds a weak pointer to a collector.WeakDispatchcan be created viaDispatch::downgrade, and can be transformed into aDispatchviaWeakDispatch::upgrade.