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

Better Debug for Mutex #2725

Merged
merged 4 commits into from
Jul 31, 2020
Merged

Better Debug for Mutex #2725

merged 4 commits into from
Jul 31, 2020

Conversation

MikailBag
Copy link
Contributor

Motivation

Before it looked like

Mutex {
    s: Semaphore,
    d: UnsafeCell
}

which is quite useless.
New output is

Mutex {
    inner: X
}

Where X is determined by locking the mutex. If we succeed, we call Debug for inner valuer. Otherwise, X := "<locked>".

IMO proposed Debug output is strictly better than current.

Alternative: always lock mutex and show inner value. However, such behavior can be rather unexpected so I implemented something more conservative.

@MikailBag MikailBag changed the title Sync debug Better Debug for Mutex Jul 30, 2020
@Darksonn Darksonn added A-tokio Area: The main tokio crate C-enhancement Category: A PR with an enhancement or bugfix. M-sync Module: tokio/sync labels Jul 30, 2020
@MikailBag
Copy link
Contributor Author

Same should be done for RwLock, but it misses try_read method currently.

Copy link
Member

@hawkw hawkw left a comment

Choose a reason for hiding this comment

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

I'm definitely in favor of making the formatted output more user-friendly.

I had a couple notes on changes we might want to make for consistency with how std::sync::Mutex implements Debug. What do you think?

tokio/src/sync/mutex.rs Outdated Show resolved Hide resolved
tokio/src/sync/mutex.rs Outdated Show resolved Hide resolved
@MikailBag
Copy link
Contributor Author

@hawkw thanks for your review! I've made both improvements and added a test.

Copy link
Member

@hawkw hawkw left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@Darksonn Darksonn merged commit 8fda719 into tokio-rs:master Jul 31, 2020
@MikailBag MikailBag deleted the sync-debug branch July 31, 2020 19:45
@Darksonn Darksonn mentioned this pull request Nov 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tokio Area: The main tokio crate C-enhancement Category: A PR with an enhancement or bugfix. M-sync Module: tokio/sync
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants