-
Notifications
You must be signed in to change notification settings - Fork 726
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
Labels
crate/appender
Related to the `tracing-appender` crate.
Comments
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
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 beSend + Sync + 'static
. TheSend + 'static
bounds make sense as the writer is given to a spawned worker thread, but I'm not sure why it'sSync
. It doesn't occur in the type signature or fields of any of thetracing_appender::non_blocking
types and so this bound is not necessary to make those typesSync
, 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.The text was updated successfully, but these errors were encountered: