Skip to content

std: Always check for EDEADLK in rwlocks on unix #25015

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

Merged
merged 2 commits into from
May 2, 2015

Conversation

alexcrichton
Copy link
Member

Apparently implementations are allowed to return EDEADLK instead of blocking
forever, in which case this can lead to unsafety in the RwLock primitive
exposed by the standard library. A debug-build of the standard library would
have caught this error (due to the debug assert), but we don't ship debug
builds right now.

This commit adds explicit checks for the EDEADLK error code and triggers a panic
to ensure the call does not succeed.

Closes #25012

Apparently implementations are allowed to return EDEADLK instead of blocking
forever, in which case this can lead to unsafety in the `RwLock` primitive
exposed by the standard library. A debug-build of the standard library would
have caught this error (due to the debug assert), but we don't ship debug
builds right now.

This commit adds explicit checks for the EDEADLK error code and triggers a panic
to ensure the call does not succeed.

Closes rust-lang#25012
@rust-highfive
Copy link
Contributor

r? @nikomatsakis

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

@alexcrichton
Copy link
Member Author

r? @aturon

@rust-highfive rust-highfive assigned aturon and unassigned nikomatsakis Apr 30, 2015
@aturon
Copy link
Member

aturon commented May 1, 2015

Fun!

@bors: r+

@bors
Copy link
Collaborator

bors commented May 1, 2015

📌 Commit 5c8ca26 has been approved by aturon

@huonw
Copy link
Member

huonw commented May 1, 2015

Do we have any test builders that bootstrap/test with debug assertions on in std/rustc?

@sfackler
Copy link
Member

sfackler commented May 1, 2015

Is there any reason to not simply change the debug_assert_eq to assert_eq at this point?

@bors
Copy link
Collaborator

bors commented May 1, 2015

⌛ Testing commit 5c8ca26 with merge 071c62d...

@bors
Copy link
Collaborator

bors commented May 1, 2015

⛄ The build was interrupted to prioritize another pull request.

@bors
Copy link
Collaborator

bors commented May 1, 2015

⌛ Testing commit 5c8ca26 with merge 9ca199a...

@bors
Copy link
Collaborator

bors commented May 1, 2015

⛄ The build was interrupted to prioritize another pull request.

@bors
Copy link
Collaborator

bors commented May 1, 2015

⌛ Testing commit 5c8ca26 with merge 47b0298...

@bors
Copy link
Collaborator

bors commented May 1, 2015

⛄ The build was interrupted to prioritize another pull request.

@alexcrichton
Copy link
Member Author

@huonw

Do we have any test builders that bootstrap/test with debug assertions on in std/rustc?

Currently, no


@sfackler

Is there any reason to not simply change the debug_assert_eq to assert_eq at this point?

Only in the sense that you get a nicer error message

bors added a commit that referenced this pull request May 2, 2015
Apparently implementations are allowed to return EDEADLK instead of blocking
forever, in which case this can lead to unsafety in the `RwLock` primitive
exposed by the standard library. A debug-build of the standard library would
have caught this error (due to the debug assert), but we don't ship debug
builds right now.

This commit adds explicit checks for the EDEADLK error code and triggers a panic
to ensure the call does not succeed.

Closes #25012
@bors
Copy link
Collaborator

bors commented May 2, 2015

⌛ Testing commit 5c8ca26 with merge b858b7f...

@bors bors merged commit 5c8ca26 into rust-lang:master May 2, 2015
@alexcrichton alexcrichton deleted the rwlock-check-ret branch May 5, 2015 17:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rwlock handles recursive locking attempts poorly on linux (double unlock)
7 participants