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

NonBlocking requires its inner writer to be Sync and I'm not sure why #1934

Closed
lilyball opened this issue Feb 17, 2022 · 0 comments · Fixed by #2607
Closed

NonBlocking requires its inner writer to be Sync and I'm not sure why #1934

lilyball opened this issue Feb 17, 2022 · 0 comments · Fixed by #2607
Labels
crate/appender Related to the `tracing-appender` crate.

Comments

@lilyball
Copy link
Contributor

Bug Report

Version

tracing-appender master (according to tracing.rs) and 0.2.0 (according to docs.rs)

Crates

tracing-appender

Description

tracing_appender::non_blocking::NonBlocking requires its wrapped writer to be Send + Sync + 'static. The Send + 'static bounds make sense as the writer is given to a spawned worker thread, but I'm not sure why it's Sync. It doesn't occur in the type signature or fields of any of the tracing_appender::non_blocking types and so this bound is not necessary to make those types Sync, and I don't see any way in which access to the writer is shared across threads. It's moved into the spawned thread and accessed exclusively from there until the thread shuts down.

@hawkw hawkw added the crate/appender Related to the `tracing-appender` crate. label Feb 17, 2022
hawkw pushed a commit that referenced this issue Oct 15, 2023
## Motivation

`NonBlocking` from `tracing-appender` wraps a writer and requires that
writer to implement `Sync` (among other bounds). However it is not clear
why this bound is necessary in first place: this writer is sent to a
dedicated thread and never used directly concurrently.

#1934 demonstrates that it is a real problem for some people. Also at my
current work we hit this issue as well when a third-party writer did not
implement `Sync`. Our workaround was to wrap that writer in our own type
and manually implement `Send` and `Sync` for it. Needless to say that it
is more a hack than a proper solution.

## Solution

Remove `Sync` bound in relevant places. Yep, that simple. Probably
having it in first place was just an oversight.

Closes #1934
davidbarsky pushed a commit that referenced this issue Nov 7, 2023
## Motivation

`NonBlocking` from `tracing-appender` wraps a writer and requires that
writer to implement `Sync` (among other bounds). However it is not clear
why this bound is necessary in first place: this writer is sent to a
dedicated thread and never used directly concurrently.

#1934 demonstrates that it is a real problem for some people. Also at my
current work we hit this issue as well when a third-party writer did not
implement `Sync`. Our workaround was to wrap that writer in our own type
and manually implement `Send` and `Sync` for it. Needless to say that it
is more a hack than a proper solution.

## Solution

Remove `Sync` bound in relevant places. Yep, that simple. Probably
having it in first place was just an oversight.

Closes #1934
hawkw pushed a commit that referenced this issue Nov 7, 2023
## Motivation

`NonBlocking` from `tracing-appender` wraps a writer and requires that
writer to implement `Sync` (among other bounds). However it is not clear
why this bound is necessary in first place: this writer is sent to a
dedicated thread and never used directly concurrently.

#1934 demonstrates that it is a real problem for some people. Also at my
current work we hit this issue as well when a third-party writer did not
implement `Sync`. Our workaround was to wrap that writer in our own type
and manually implement `Send` and `Sync` for it. Needless to say that it
is more a hack than a proper solution.

## Solution

Remove `Sync` bound in relevant places. Yep, that simple. Probably
having it in first place was just an oversight.

Closes #1934
kaffarell pushed a commit to kaffarell/tracing that referenced this issue Nov 21, 2023
…#2607)

## Motivation

`NonBlocking` from `tracing-appender` wraps a writer and requires that
writer to implement `Sync` (among other bounds). However it is not clear
why this bound is necessary in first place: this writer is sent to a
dedicated thread and never used directly concurrently.

tokio-rs#1934 demonstrates that it is a real problem for some people. Also at my
current work we hit this issue as well when a third-party writer did not
implement `Sync`. Our workaround was to wrap that writer in our own type
and manually implement `Send` and `Sync` for it. Needless to say that it
is more a hack than a proper solution.

## Solution

Remove `Sync` bound in relevant places. Yep, that simple. Probably
having it in first place was just an oversight.

Closes tokio-rs#1934
kaffarell pushed a commit to kaffarell/tracing that referenced this issue Feb 14, 2024
…#2607)

## Motivation

`NonBlocking` from `tracing-appender` wraps a writer and requires that
writer to implement `Sync` (among other bounds). However it is not clear
why this bound is necessary in first place: this writer is sent to a
dedicated thread and never used directly concurrently.

tokio-rs#1934 demonstrates that it is a real problem for some people. Also at my
current work we hit this issue as well when a third-party writer did not
implement `Sync`. Our workaround was to wrap that writer in our own type
and manually implement `Send` and `Sync` for it. Needless to say that it
is more a hack than a proper solution.

## Solution

Remove `Sync` bound in relevant places. Yep, that simple. Probably
having it in first place was just an oversight.

Closes tokio-rs#1934
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
crate/appender Related to the `tracing-appender` crate.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants