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

Fix potential deadlock #59

Merged
merged 1 commit into from
Sep 27, 2023
Merged

Fix potential deadlock #59

merged 1 commit into from
Sep 27, 2023

Conversation

AsmPrgmC3
Copy link
Contributor

Motivation

Some Debug implementations access a span's extensions, for example a color_eyre Report when printing a Spantrace.
This can currently lead to a deadlock because tracing-opentelemetry's tracing Layer holds an extensions_mut() guard for its Span while recording the event.

Solution

Move the the OtelData out of the span to not hold the lock and later write the extension back.

@AsmPrgmC3 AsmPrgmC3 requested a review from jtescher as a code owner September 5, 2023 22:50
@asonix
Copy link

asonix commented Sep 6, 2023

Probably related to: tokio-rs/tracing#1565

Copy link
Collaborator

@jtescher jtescher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for looking into this, does not appear to have a negative perf impact on existing benchmarks which is a little surprising!

@jtescher jtescher merged commit 1c61ea6 into tokio-rs:v0.1.x Sep 27, 2023
jtescher added a commit that referenced this pull request Nov 7, 2023
### Breaking Changes

- Upgrade to `v0.21.0` of `opentelemetry` For list of breaking changes
in OpenTelemetry, see the [v0.21.0
changelog](https://github.com/open-telemetry/opentelemetry-rust/blob/v0.21.0/opentelemetry/CHANGELOG.md).
- Update MSRV to require Rust 1.65+, as `opentelemetry` requires it now.
(#68)

### Fixed

- WASM Support (#57)
- Fix potential deadlock (#59)
mladedav added a commit to mladedav/tracing-opentelemetry that referenced this pull request Feb 17, 2024
This deadlock can happen when extensions of a span are locked while we
visit either an event or attributes of a span. A `Debug` implementation
can also try to access the extensions which results in a deadlock.

See tokio-rs#59 and tokio-rs#95 for additional information.
jtescher pushed a commit that referenced this pull request Feb 26, 2024
## Motivation

#94 - Mutli-threaded tracing drops most of the events

## Solution

I have reorganized the code so that the lock on extensions is held, but
not while `record` is called which should solve the deadlock originally
solved in #59

### Benchmark

I've used the benchmark from #93, please ignore the "filtered" vs
"non_filtered" distinction, for the purpose of this PR they should be
identical cases.

See results in later comment as the code got changed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants