dotnet / dotnet/wpf

Infinite spin wait in CWGXBitmapLockState::LockRead()

Open
#9,088 2 comments 1 reaction 0 assignees View on GitHub
Investigate
Dominant language
C#
Stars
7.7k
Forks
1.3k
Avg merge
1d 11h
Merged PRs (30d)
61

Description

### Description

In method [CWGXBitmapLockState::LockRead()](https://github.com/dotnet/wpf/blob/2af6ee77ff756fde694b0695ccda03e44403a72e/src/Microsoft.DotNet.Wpf/src/WpfGfx/common/scanop/bitmap.cpp#L1063), the exit-on-other-lock-active is implemented incorrectly.

```
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_

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.