Test(lib/sync): Fix test_rwlock_max_readers for x86 Win7#154967
Test(lib/sync): Fix test_rwlock_max_readers for x86 Win7#154967PaulDance wants to merge 1 commit intorust-lang:mainfrom
test_rwlock_max_readers for x86 Win7#154967Conversation
rustbot
added
S-waiting-on-review
|
rustbot has assigned @Mark-Simulacrum. Use Why was this reviewer chosen?The reviewer was selected based on:
|
rustbot
added
A-atomic
|
I think unless we have evidence we should do something different, keeping the modification to just windows for now would make sense to me. I'm happy we're uncovering these differences though! I'm not in practice too worried about the deadlock (though a panic feels probably better), especially on 32-bit Windows, on this high a value. |
The recently-added test currently systematically deadlocks when running it under i686 Windows 7, but not x86_64 that passes it fine. This therefore fixes the test for the target. Empirically, the correct value for `MAX_READERS` seems to be `2^28 - 1`: removing the `- 1` re-introduces the deadlock, at least under our testing environment. This fix thus uses this value. However, I have no real justification to support that, because I find myself a bit at a loss when comparing the implementation details, the comment added above the test and what the current value is; some help would therefore be nice in this aspect. Also, the value change is restricted to 32-bit Win7 as there is no evidence to support it should be done for other targets. Signed-off-by: Paul Mabileau <paul.mabileau@harfanglab.fr>
PaulDance
force-pushed
the
patches/fix-x86-win7-rwlock-max-test
branch
from
ecebc8f to
962e9e2
Compare
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
Indeed, done. |
3 participants
The test recently added in #153555 currently systematically deadlocks when running it under i686 Windows 7, but not x86_64 that passes it fine. This therefore fixes the test for the target.
Empirically, the correct value for
MAX_READERSseems to be2^28 - 1: removing the- 1re-introduces the deadlock, at least under our testing environment. This fix thus uses this value. However, I have no real justification to support that, because I find myself a bit at a loss when comparing the implementation details, the comment added above the test and what the current value is; some help would therefore be nice in this aspect. Also, the value change is restricted to 32-bit Win7 as there is no evidence to support it should be done for other targets.cc @roblabla
@rustbot label O-windows-msvc O-windows-7 O-x86_32 A-atomic T-libs