Add MemoryBarrier to SyncBlock Lock upgrade path #121355
Conversation
jkoritzinsky
added
area-VM-coreclr
NO-MERGE
|
Tagging subscribers to this area: @mangod9 |
|
/azp run runtime-coreclr gcstress-extra |
|
Azure Pipelines successfully started running 1 pipeline(s). |
…that other threads observe the swapped-in and initialized lock before the thin-lock value is cleared.
jkoritzinsky
force-pushed
the
monitor-fix-testing
branch
from
b9ea6b5 to
ab31426
Compare
|
/azp run runtime-coreclr gcstress-extra |
|
Azure Pipelines successfully started running 1 pipeline(s). |
jkoritzinsky
changed the title
jkoritzinsky
removed
NO-MERGE
There was a problem hiding this comment.
Pull Request Overview
This PR adds a memory barrier after successfully storing a lock handle in a SyncBlock to ensure proper memory ordering when transitioning from thin lock to full lock semantics.
- Adds an explicit
MemoryBarrier()call after the successfulInterlockedCompareExchangeToperation inGetOrCreateLock
…GetOrCreateLock instead of trying to do something lock-free.
…teresting thin lock bits.
jkotas
left a comment
jkotas
left a comment
There was a problem hiding this comment.
Could you please trigger a few GC stress runs to see whether it is fixing the problem (and not introducing any new ones)?
jkoritzinsky
force-pushed
the
monitor-fix-testing
branch
from
44ff651 to
880afbc
Compare
|
/azp run runtime-coreclr gcstress-extra, runtime-coreclr gcstress0x3-gcstress0xc |
|
Azure Pipelines successfully started running 2 pipeline(s). |
|
Looks like I need to modify the timeout handling, which makes sense (especially if a GC happens). |
…ht get a GC on the thread that owns the lock, so spinning for a while makes sense. Also remove more store barriers.
|
/azp run runtime-coreclr gcstress-extra, runtime-coreclr gcstress0x3-gcstress0xc |
|
Azure Pipelines successfully started running 2 pipeline(s). |
|
/ba-g android timeouts unrelated |
2 participants
Fixes #121287
Validation: Every run of runtime-coreclr gcstress-extra has at least one hit of the failure in the link for some out-of-proc test. This PR has no failures of the sort. This is not my favorite form of validation, but I don't think I'm going to get much better as I can't repro locally.
May also fix #121285, but I'm not sure.