Bug: Reentracy checks conflict with .Net synchronization mechanisms (e.g. lock), causing random crashes in apps
Nobody has claimed this yet.
- Dominant language
- C#
- Stars
- 665
- Forks
- 134
- Avg merge
- 1d 3h
- Merged PRs (30d)
- 32
Description
Description
The issue is explained very clearly here - https://github.com/microsoft/microsoft-ui-xaml/issues/11099.
Basically, the problem is that all .Net synchronization primitives, including lock, when used on an STA thread, will eventually call CoWaitForMultipleHandles, which pumps messages. Unfortunately the WindowsAppSdk windowing code (xcpwindow.cpp) doesn't deal with reentrancy very well (just fails fast in a lot of cases), resulting in the app sometimes crashing when any contended lock is encountered (which could even be in CsWinRT code itself). Obviously we need to be able to use locks, and this is quite a critical bug. It seems like there are 2 solutions. The first is for WindowsAppSDK/WinUI to update their code to properly handle reentrancy (at least in the case of messages that can be pumped by CoWaitForMultipleHandles). If this is the preferred option then perhaps you guys can put some pressure on the WindowsAppSDK/WinUI team to fix the issue. The second option is to use a custom synchronization context which overrides Wait to use a non-re-entrant wait (some example code will follow). If this is the case it would involve an update to DispatcherQueueSynchronizationContext (when used on an STA thread). But I'm not sure if this would have any unintended consequnces...
Here is a custom synchronization context that fixes the issue (note it doesn't necessarily handle errors etc correctly at the moment).
internal partial class CustomSynchronizationContext : DispatcherQueueSynchronizationContext {
public CustomSynchronizationContext(DispatcherQueue queue) : base(queue) {
SetWaitNotificationRequired();
}
public override int Wait(nint[] waitHandles, bool waitAll, int millisecondsTimeout) {
return (int)WaitForMultipleObjectsEx((uint)waitHandles.Length, waitHandles, waitAll, (uint)millisecondsTimeout, false); //Note the (uint)millisecondsTimeout cast correctly handles the -1 case
}
[LibraryImport("Kernel32.dll")]
private static partial uint WaitForMultipleObjectsEx(uint nCount, nint[] lpHandles, [MarshalAs(UnmanagedType.Bool)] bool bWaitAll, uint dwMilliseconds, [MarshalAs(UnmanagedType.Bool)] bool bAlertable);
}
Steps To Reproduce
The minimal code needed to reproduce the crash is as follows (also see the sample solution in https://github.com/microsoft/microsoft-ui-xaml/issues/11099.
public sealed partial class MainWindow : Window {
readonly object lockObj = new object();
public MainWindow() {
InitializeComponent();
CompositionTarget.Rendering += CompositionTarget_Rendering;
Task.Run(() => {
while (true) {
lock (lockObj) { //Ensures contention at some point
}
}
});
}
private void CompositionTarget_Rendering(object? sender, object e) {
lock (lockObj) {
}
}
}
<?xml version="1.0" encoding="utf-8"?>
<Window x:Class="TestLockReentrancy.MainWindow" xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation" xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml" xmlns:local="using:TestLockReentrancy" Title="TestLockReentrancy">
<Grid Background="Gray" ManipulationMode="All">
</Grid>
</Window>
Note the important ManipulationMode="All". Run the app, randomly drag and release the mouse in the grey area until there is a crash.
Expected Behavior
No crash
Version Info
WindowsAppSdk 2.1, Windows SDK 26100, Windows 25H2.
Additional Context
No response
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with DispatcherQueueSynchronizationContext and the C# reproduction's MainWindow/CompositionTarget.Rendering path; compare its behavior with the referenced WindowsAppSDK xcpwindow.cpp reentrancy path. Reproduce with lock contention and ManipulationMode="All", then validate a chosen fix so contended STA locks no longer crash.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- csharp
- Domain
- desktop
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 45/100