Skip to content

{Mutex,RwLock}::get_mut() documentation does not consider if a guard is forgotten #139034

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

Closed
kpreid opened this issue Mar 27, 2025 · 1 comment · Fixed by #139164
Closed

{Mutex,RwLock}::get_mut() documentation does not consider if a guard is forgotten #139034

kpreid opened this issue Mar 27, 2025 · 1 comment · Fixed by #139164
Assignees
Labels
A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools T-libs Relevant to the library team, which will review and decide on the PR/issue.

Comments

@kpreid
Copy link
Contributor

kpreid commented Mar 27, 2025

Location

https://doc.rust-lang.org/std/sync/struct.Mutex.html#method.get_mut
https://doc.rust-lang.org/std/sync/struct.RwLock.html#method.get_mut
https://doc.rust-lang.org/std/cell/struct.RefCell.html#method.get_mut

Summary

The documentation of Mutex says this (and similar wording exists in RefCell and RwLock):

Since this call borrows the Mutex mutably, no actual locking needs to take place – the mutable borrow statically guarantees no locks exist.

However, the last part is not quite accurate. A mutex guard may be forgotten, in which case get_mut() still succeeds, but the Mutex is in fact still locked. think it’s worth addressing this point because:

  • One may wonder if this function is sound, given its documentation seeming to believe forgetting doesn’t exist.

  • The implementation of get_mut() could, in addition to returning a reference, reset the mutex to be unlocked so it can be accessed in the normal fashion in the future. That doesn’t happen in at least one current implementation,

    let mut m = std::sync::Mutex::new(1);
    std::mem::forget(m.lock().unwrap());
    *m.get_mut().unwrap() = 2;
    *m.lock().unwrap() // deadlocks

    and I doubt it ever would, but it’s an observable choice of behavior that it doesn’t. (RefCell has RefCell::undo_leak() which actually does this, but neither Mutex nor RwLock has an analogous function.)

A more accurate but cryptic wording would be to replace “guarantees no locks exist” would be “guarantees no usable MutexGuards exist”; or, an entire paragraph/section about forgetting could be added. A decision needs to be made about whether the documentation promises that the mutex will always still be locked, or whether that is an implementation detail subject to change.

@kpreid kpreid added the A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools label Mar 27, 2025
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Mar 27, 2025
@xizheyin
Copy link
Contributor

@rustbot claim

@bors bors closed this as completed in d5f930f Apr 10, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Apr 10, 2025
Rollup merge of rust-lang#139164 - xizheyin:issue-139034, r=joboet

std: improve documentation for get_mut() methods regarding forgotten guards

Fixes rust-lang#139034

This PR improves the documentation for `get_mut()` methods in `Mutex`, `RefCell`, and `RwLock` to clarify their behavior when lock guards are forgotten (e.g., via std::mem::forget).

The current documentation for these methods states that a mutable borrow "statically guarantees no locks exist", which is not entirely accurate. While a mutable borrow prevents new locks from being created, it does not clear or detect previously abandoned locks through `forget()`. This can lead to counterintuitive behavior:

https://play.rust-lang.org/?version=stable&mode=debug&edition=2024&gist=e68cefec12dcd435daf2237c16824ed3
https://play.rust-lang.org/?version=nightly&mode=debug&edition=2024&gist=81263ad652c752afd63c903113d3082c
https://play.rust-lang.org/?version=stable&mode=debug&edition=2024&gist=311baa4edb3abf82a25c8d7bf21a4a52

r? libs
@jieyouxu jieyouxu added T-libs Relevant to the library team, which will review and decide on the PR/issue. and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Apr 10, 2025
github-actions bot pushed a commit to model-checking/verify-rust-std that referenced this issue Apr 19, 2025
std: improve documentation for get_mut() methods regarding forgotten guards

Fixes rust-lang#139034

This PR improves the documentation for `get_mut()` methods in `Mutex`, `RefCell`, and `RwLock` to clarify their behavior when lock guards are forgotten (e.g., via std::mem::forget).

The current documentation for these methods states that a mutable borrow "statically guarantees no locks exist", which is not entirely accurate. While a mutable borrow prevents new locks from being created, it does not clear or detect previously abandoned locks through `forget()`. This can lead to counterintuitive behavior:

https://play.rust-lang.org/?version=stable&mode=debug&edition=2024&gist=e68cefec12dcd435daf2237c16824ed3
https://play.rust-lang.org/?version=nightly&mode=debug&edition=2024&gist=81263ad652c752afd63c903113d3082c
https://play.rust-lang.org/?version=stable&mode=debug&edition=2024&gist=311baa4edb3abf82a25c8d7bf21a4a52

r? libs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants