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

Warn about Mutex<()> #4471

Open
oli-cosmian opened this issue Aug 29, 2019 · 1 comment
Open

Warn about Mutex<()> #4471

oli-cosmian opened this issue Aug 29, 2019 · 1 comment
Labels
A-lint Area: New lints L-correctness Lint: Belongs in the correctness lint group

Comments

@oli-cosmian
Copy link
Contributor

I can't think of a use case where this actually gives you any guarantees. You can still use it to protect another Sync data structure, but you can just as well forget to do so. One should always put the components that are protected into the Mutex even if not strictly necessary due to the componentes being thread safe themselves.

An example of such a situation would be

struct Foo {
    lock: Mutex<()>,
    a: AtomicUsize,
    b: AtomicUsize,
}

where one should not modify the two values one by one, but always together in a single critical section.

@flip1995 flip1995 added L-correctness Lint: Belongs in the correctness lint group A-lint Area: New lints labels Aug 30, 2019
@Arnavion
Copy link
Contributor

Arnavion commented Sep 6, 2019

There is a use case for Mutex<()>. Consider that you have three Rust functions that wrap C functions provided by a non-reentrant C lib, so all three functions need to synchronize between themselves. They can do this by being inherent fns of a newtype struct CHandle(Mutex<()>);, since they do need a Mutex for unlock notifications, but there's nothing to guard inside the mutex.

Similarly if you many disparate types whose inherent fns are implemented in terms of the non-reentrant C library's functions, then multiple disparate values might end up sharing the same Arc<CHandle>.

But this does not detact from the lint proposal, since Mutex<()> misuse is definitely something to lint for. The cases I described can easily #[allow] the lint.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: New lints L-correctness Lint: Belongs in the correctness lint group
Projects
None yet
Development

No branches or pull requests

3 participants