-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Infinite spin wait in CWGXBitmapLockState::LockRead() #9088
Comments
If you don't clear the bit, then the count would be wrong... a better fix would be to do |
Since m_lockState is being used with atomic updates, I think the idea is to only read m_lockState once, and then use InterlockedWhatever to update it. m_lockState will be one of three things:
If it's currently 0x8000000, then incCount will be 0x80000001, which will properly trigger the "already locked" error return. m_lockState will never be incremented to 0x80000001; incCount will never be 0x80000002. If it's zero or a small number, then incCount will be a small number, and it won't error return. If somehow we end up with 2 billion simultaneous read locks, then m_LockState will be 0x7FFFFFFF, incCount will be 0x8000000, and the "too many reads" half of the error return will happen. But more importantly, it absolutely must change to this:
If lockCount is not equal to the old value of m_lockState, then InterlockedCompareExchange will always fail, which is what's causing the endless loop that I've been seeing. |
Description
In method CWGXBitmapLockState::LockRead(), the exit-on-other-lock-active is implemented incorrectly.
This is incorrect. Because the high bit is cleared when reading the lock, the "LockRead failed -- outstanding write lock" will never happen. Then, because lockCount is not the same value as m_lockState, the compare exchange will fail, and it will loop. If there is an outstanding write lock, this will loop forever. (In my application, this ended up deadlocking with the UI thread.)
Solution: Don't clear the high bit.
Reproduction Steps
Sorry, I don't have a demo application that exhibits this all the time.
I was able to get a deadlock between the render thread and the UI thread: The render thread was stuck in this method, the UI thread was in DUCE.Channel.SyncFlush() --> CMilChannel::SyncFlush() --> CMilConnection::SynchronizeChannel(hChannel), waiting on a synchronization object that presumably would have been signaled from the render thread.
Expected behavior
As the comments in the method say, I expect it to exit with an error if a write lock is active.
Actual behavior
It doesn't.
Regression?
No response
Known Workarounds
No response
Impact
No response
Configuration
.Net version: seen in .Net 7 and .Net 8. Also present in main branch in this repo.
Other information
No response
The text was updated successfully, but these errors were encountered: