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

Add Sync bound for set_global_recorder #513

Merged
merged 1 commit into from
Sep 14, 2024
Merged

Conversation

Nekrolm
Copy link
Contributor

@Nekrolm Nekrolm commented Sep 6, 2024

global recorder is shared between threads and must be sync

closes #511

@Nekrolm
Copy link
Contributor Author

Nekrolm commented Sep 6, 2024

I spend some time thinking, if the Send bound is also needed. But seems not:

https://github.com/metrics-rs/metrics/blob/main/metrics/src/recorder/cell.rs#L44

global recorder is leaked via Box::leak and its destructor won't be called

global recorder is shared between threads and must be sync

closes metrics-rs#511
@@ -55,7 +55,7 @@ pub struct RecoverableRecorder<R> {
handle: Arc<R>,
}

impl<R: Recorder + 'static> RecoverableRecorder<R> {
impl<R: Recorder + Sync + Send + 'static> RecoverableRecorder<R> {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Send enforced here by Arc

@tobz tobz merged commit acd341c into metrics-rs:main Sep 14, 2024
12 checks passed
@tobz tobz added C-core Component: core functionality such as traits, etc. E-intermediate Effort: intermediate. T-bug Type: bug. S-awaiting-release Status: awaiting a release to be considered fixed/implemented. labels Sep 14, 2024
@tobz
Copy link
Member

tobz commented Oct 12, 2024

Released as part of [email protected]/[email protected].

Thanks again for your contribution!

@tobz tobz removed the S-awaiting-release Status: awaiting a release to be considered fixed/implemented. label Oct 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-core Component: core functionality such as traits, etc. E-intermediate Effort: intermediate. T-bug Type: bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

set_global_recorder should apply Sync bound
2 participants