-
Notifications
You must be signed in to change notification settings - Fork 717
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: Entered
guards should be !Send
#698
Labels
crate/tracing
Related to the `tracing` crate
kind/bug
Something isn't working
meta/breaking
This is a breaking change, and should wait until the next breaking release.
Milestone
Comments
hawkw
added
kind/bug
Something isn't working
crate/tracing
Related to the `tracing` crate
meta/breaking
This is a breaking change, and should wait until the next breaking release.
labels
Apr 28, 2020
6 tasks
hawkw
added a commit
that referenced
this issue
Sep 29, 2020
The `Entered` guard returned by `Span::enter` represents entering and exiting a span _on the current thread_. Calling `Span::enter` enters the span, returning an `Entered`, and dropping the `Entered` ensures that the span is exited. This ensures that all spans, once entered, are eventually exited. However, `Entered` has an auto-impl of `Send`, because it doesn't contain any `!Send` types. This means that the `Entered` guard _could_ be sent to another thread and dropped. This would cause the original thread to never exit the span,. and the thread to which the `Entered` guard was sent would exit a span that it never observed an enter for. This is incorrect. This PR adds a `*mut ()` field to `Entered` so that it no longer implements `Send`. There's now a manual `Sync` impl so structs holding an `Entered` can still be `Sync`. Fixes #698 Signed-off-by: Eliza Weisman <eliza@buoyant.io>
hawkw
added a commit
that referenced
this issue
Sep 30, 2020
## Motivation The `Entered` guard returned by `Span::enter` represents entering and exiting a span _on the current thread_. Calling `Span::enter` enters the span, returning an `Entered`, and dropping the `Entered` ensures that the span is exited. This ensures that all spans, once entered, are eventually exited. However, `Entered` has an auto-impl of `Send`, because it doesn't contain any `!Send` types. This means that the `Entered` guard _could_ be sent to another thread and dropped. This would cause the original thread to never exit the span,. and the thread to which the `Entered` guard was sent would exit a span that it never observed an enter for. This is incorrect. ## Solution This PR adds a `*mut ()` field to `Entered` so that it no longer implements `Send`. There's now a manual `Sync` impl so structs holding an `Entered` can still be `Sync`. Fixes #698 Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
crate/tracing
Related to the `tracing` crate
kind/bug
Something isn't working
meta/breaking
This is a breaking change, and should wait until the next breaking release.
Bug Report
Version
Crates
tracing
Description
The
Entered
guard returned bySpan::enter
represents entering and exiting a span on the current thread. CallingSpan::enter
enters the span, returning anEntered
, and dropping theEntered
ensures that the span is exited. This ensures that all spans, once entered, are eventually exited.However,
Entered
has an auto-impl ofSend
, because it doesn't contain any!Send
types. This means that theEntered
guard could be sent to another thread and dropped. This would cause the original thread to never exit the span,. and the thread to which theEntered
guard was sent would exit a span that it never observed an enter for. This is incorrect.We should consider making
Entered
!Send
. Technically, this is a breaking change, but the current behavior is incorrect. This would also preventEntered
guards from being held in futures that are bounded withSend
, which would prevent some incorrect uses ofSpan::enter
inasync
blocks...The text was updated successfully, but these errors were encountered: