Skip to content

Unix Rwlock write_contended code issue #97162

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
Warrenren opened this issue May 19, 2022 · 2 comments · Fixed by #97245
Closed

Unix Rwlock write_contended code issue #97162

Warrenren opened this issue May 19, 2022 · 2 comments · Fixed by #97245
Assignees
Labels
C-bug Category: This is a bug.

Comments

@Warrenren
Copy link
Contributor

I have review the following code for a lot times:
the code position: /library/std/src/sys/unix/locks/futex_rwlock.rs : line 212

    fn write_contended(&self) {
            ...
            ...
            
            let s = self.state.load(Relaxed);
            // I guess "is_unlocked(state)" should be "is_unlocked(s)"
           // because here "state" should be locked 
          //  and the code should  test "s" for the current status
            if is_unlocked(state) || !has_writers_waiting(s) {
                state = s;
                continue;
            }

            ...
            ...
    }

the code should test the current "self.state" again to avoid unlock write. but "state" is old and is tested. I can not get what it does.

Meta

rustc --version --verbose:

rustc 1.62.0-nightly (52ca603da 2022-04-12)
binary: rustc
commit-hash: 52ca603da73ae9eaddf96f77953b33ad8c47cc8e
commit-date: 2022-04-12
host: x86_64-pc-windows-msvc
release: 1.62.0-nightly
LLVM version: 14.0.0
Backtrace

<backtrace>

@Warrenren Warrenren added the C-bug Category: This is a bug. label May 19, 2022
@RalfJung
Copy link
Member

Cc @m-ou-se

@m-ou-se m-ou-se self-assigned this May 21, 2022
@m-ou-se
Copy link
Member

m-ou-se commented May 21, 2022

Thanks! That looks like a mistake. I'll fix it.

However, luckily, I believe this doesn't affect the correctness of the lock. If the if-statement was just

if !has_writers_waiting(s) {

it still behaves correctly.

@bors bors closed this as completed in 76725e0 May 22, 2022
workingjubilee pushed a commit to tcdi/postgrestd that referenced this issue Sep 15, 2022
Fix typo in futex RwLock::write_contended.

I wrote `state` where I should've used `s`.

This was spotted by `@Warrenren.`

This change removes the unnecessary `s` variable to prevent that mistake.

Fortunately, this typo didn't affect the correctness of the lock, as the
second half of the condition (!has_writers_waiting) is enough for
correctness, which explains why this mistake didn't show up during
testing.

Fixes rust-lang/rust#97162
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants