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

Assert that pthread mutex initialization succeeded #77761

Merged
merged 1 commit into from
Oct 20, 2020

Conversation

tmiasko
Copy link
Contributor

@tmiasko tmiasko commented Oct 9, 2020

If pthread mutex initialization fails, the failure will go unnoticed unless
debug assertions are enabled. Any subsequent use of mutex will also silently
fail, since return values from lock & unlock operations are similarly checked
only through debug assertions.

In some implementations the mutex initialization requires a memory
allocation and so it does fail in practice.

Assert that initialization succeeds to ensure that mutex guarantees
mutual exclusion.

Fixes #34966.

@rust-highfive
Copy link
Collaborator

r? @cramertj

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 9, 2020
@tmiasko
Copy link
Contributor Author

tmiasko commented Oct 9, 2020

r? @cuviper

@rust-highfive rust-highfive assigned cuviper and unassigned cramertj Oct 9, 2020
@cuviper
Copy link
Member

cuviper commented Oct 19, 2020

I think it's a good idea to enforce this, but the assertion will not be very meaningful to a user. Your change from result to r will be less meaningful in the panic message, and a user has to be a bit savvy to know that this is a POSIX error number.

Maybe they should use cvt_nz instead? Then we can unwrap it for a formatted error message, either directly here or from the next caller level up.

@tmiasko
Copy link
Contributor Author

tmiasko commented Oct 19, 2020

Replaced with cvt_nz and unwrap, this should be informative enough coming from Mutex::init, now that we have a track-caller. I intentionally limited this to initialization for now.

@cuviper
Copy link
Member

cuviper commented Oct 19, 2020

LGTM, thanks!

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Oct 19, 2020

📌 Commit 21c29b1 has been approved by cuviper

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 19, 2020
@cuviper
Copy link
Member

cuviper commented Oct 19, 2020

I think you miscalibrated your time machine...

image

If pthread mutex initialization fails, the failure will go unnoticed unless
debug assertions are enabled. Any subsequent use of mutex will also silently
fail, since return values from lock & unlock operations are similarly checked
only through debug assertions.

In some implementations the mutex initialization requires a memory
allocation and so it does fail in practice.

Check that initialization succeeds to ensure that mutex guarantees
mutual exclusion.
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 20, 2020
Rollup of 10 pull requests

Successful merges:

 - rust-lang#77612 (BTreeMap: test invariants more thoroughly and more readably)
 - rust-lang#77761 (Assert that pthread mutex initialization succeeded)
 - rust-lang#77778 ([x.py setup] Allow setting up git hooks from other worktrees)
 - rust-lang#77838 (const keyword: brief paragraph on 'const fn')
 - rust-lang#77923 ([net] apply clippy lints)
 - rust-lang#77931 (Fix false positive for `unused_parens` lint)
 - rust-lang#77959 (Tweak ui-tests structure)
 - rust-lang#78105 (change name in .mailmap)
 - rust-lang#78111 (Trait predicate ambiguities are not always in `Self`)
 - rust-lang#78121 (Do not ICE on pattern that uses a binding multiple times in generator)

Failed merges:

r? `@ghost`
@bors bors merged commit b09ef11 into rust-lang:master Oct 20, 2020
@rustbot rustbot added this to the 1.49.0 milestone Oct 20, 2020
@tmiasko tmiasko deleted the pthread-mutex branch October 26, 2020 21:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Return codes of libc functions are checked for errors only in debug mode.
6 participants