You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
lockCount = m_lockState & (~lockWrite); // lockWrite == 0x80000000
LONG incCount = lockCount + 1;
// Check for locked for write and also extreme case of too many simultaneous read requests.
if (incCount & lockWrite)
{
TraceTag((tagMILWarning, "CWGXBitmapLockState::LockRead failed -- there is already an outstanding write lock or too many reads."));
IFC(WINCODEC_ERR_ALREADYLOCKED);
}
original = InterlockedCompareExchange(&m_lockState, incCount, lockCount);
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.
lockCount = m_lockState; // <--- No clearing high bit.
LONG incCount = lockCount + 1;
// Check for locked for write and also extreme case of too many simultaneous read requests.
if (incCount & lockWrite)
{
TraceTag((tagMILWarning, "CWGXBitmapLockState::LockRead failed -- there is already an outstanding write lock or too many reads."));
IFC(WINCODEC_ERR_ALREADYLOCKED);
}
original = InterlockedCompareExchange(&m_lockState, incCount, lockCount);
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:
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:
Zero: Not currently locked.
0x8000000: Locked for writing.
Some small number: Locked for reading, that many times.
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:
lockCount = m_lockState; // no manipulation during the read
...
original = InterlockedCompareExchange(&m_lockState, whatever, lockCount);
^^^^^^^^^ not manipulated
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: