WispLogic - Collection was modified

[marius-bughiu]

Description

Hi team, we have several crashes in our telemetry (UiPath Studio) related to WispLogic - Collection was modified; enumeration operation may not execute, and wanted to check if this is know / if you have seen this before. Googling for "WispLogic" + "Collection was modified" only returns a bug report on our forum.

Over the past 7 days we've seen this exception 790 times across our user base.

There's a couple of different stack traces associated, but in the same area:

System.InvalidOperationException: Collection was modified; enumeration operation may not execute.
at System.Collections.Generic.Dictionary`2.ValueCollection.Enumerator.MoveNext()
at System.Windows.Input.StylusWisp.WispLogic.RefreshTablets()
at System.Windows.Input.StylusWisp.WispLogic.OnTabletRemovedImpl(UInt32 wisptisIndex, Boolean isInternalCall)
at System.Windows.Input.StylusWisp.WispLogic.HandleMessage(WindowMessage msg, IntPtr wParam, IntPtr lParam)
at System.Windows.SystemResources.InvalidateTabletDevices(WindowMessage msg, IntPtr wParam, IntPtr lParam)
at System.Windows.SystemResources.SystemThemeFilterMessage(IntPtr hwnd, Int32 msg, IntPtr wParam, IntPtr lParam, Boolean& handled)
at MS.Win32.HwndWrapper.WndProc(IntPtr hwnd, Int32 msg, IntPtr wParam, IntPtr lParam, Boolean& handled)
at System.Windows.Threading.ExceptionWrapper.InternalRealCall(Delegate callback, Object args, Int32 numArgs)
at System.Windows.Threading.ExceptionWrapper.TryCatchWhen(Object source, Delegate callback, Object args, Int32 numArgs, Delegate catchHandler), HResult -2146233079

System.InvalidOperationException: Collection was modified; enumeration operation may not execute.
at System.Collections.Generic.Dictionary`2.ValueCollection.Enumerator.MoveNext()
at System.Windows.Input.StylusWisp.WispLogic.OnTabletAdded(UInt32 wisptisIndex)
at System.Windows.Input.StylusWisp.WispLogic.HandleMessage(WindowMessage msg, IntPtr wParam, IntPtr lParam)
at System.Windows.SystemResources.SystemThemeFilterMessage(IntPtr hwnd, Int32 msg, IntPtr wParam, IntPtr lParam, Boolean& handled)
at MS.Win32.HwndWrapper.WndProc(IntPtr hwnd, Int32 msg, IntPtr wParam, IntPtr lParam, Boolean& handled)
at MS.Win32.HwndSubclass.DispatcherCallbackOperation(Object o)
at System.Windows.Threading.ExceptionWrapper.InternalRealCall(Delegate callback, Object args, Int32 numArgs)
at System.Windows.Threading.ExceptionWrapper.TryCatchWhen(Object source, Delegate callback, Object args, Int32 numArgs, Delegate catchHandler), HResult -2146233079

Reproduction Steps

Cannot reproduce at the moment. These stack traces are from our app telemetry.

Expected behavior

Does not throw exception.

Actual behavior

Throws exception.

Regression?

No, it reproduces in application versions from 4 years ago.

Known Workarounds

None.

Impact

We've had 790 exception occurrences over the past 7 days across the user base.

Configuration

No response

Other information

No response

[lindexi]

@marius-bughiu It seems the thread-safe issues.

Duplicate of #7222

wpf/src/Microsoft.DotNet.Wpf/src/PresentationCore/System/Windows/Input/Stylus/Wisp/WispLogic.cs

Lines 3466 to 3480 in ee7cbb8

	private void RefreshTablets()
	{
	// rebuild all contexts and tablet collection
	foreach (PenContexts contexts in __penContextsMap.Values)
	{
	contexts.Disable(shutdownWorkerThread: false);
	}
	WispTabletDevices.UpdateTablets();
	foreach (PenContexts contexts in __penContextsMap.Values)
	{
	contexts.Enable();
	}
	}

The __penContextsMap update in RegisterHwndForInput method with the lock:

wpf/src/Microsoft.DotNet.Wpf/src/PresentationCore/System/Windows/Input/Stylus/Wisp/WispLogic.cs

Lines 3140 to 3162 in ee7cbb8

	internal void RegisterHwndForInput(InputManager inputManager, PresentationSource inputSource)
	{
	HwndSource hwndSource = (HwndSource)inputSource;
	GetAndCacheTransformToDeviceMatrix(hwndSource);
	// Keep track so we don't bother looking for changes if someone happened to query this before
	// an Avalon window was created where we get TabletAdd/Removed notification.
	bool initializedTablets = (_tabletDeviceCollection == null);
	// This causes EnableCore to be called on TabletPC systems which enabled stylus input!
	WispTabletDeviceCollection tablets = WispTabletDevices;
	lock (__penContextsLock)
	{
	if (__penContextsMap.ContainsKey(inputSource))
	{
	throw new InvalidOperationException(SR.PenService_WindowAlreadyRegistered);
	}
	PenContexts penContexts = new PenContexts(StylusLogic.GetCurrentStylusLogicAs<WispLogic>(), inputSource);
	__penContextsMap[inputSource] = penContexts;

We can only find the two case to modify the __penContextsMap: RegisterHwndForInput and UnRegisterHwndForInput

But the RegisterHwndForInput only be called in HwndStylusInputProvider..ctor . And the HwndStylusInputProvider is bind the HwndSource, which means that is will only call once in normal case.

And the UnRegisterHwndForInput only be called in Dispose. And I do not think it will cause the issues.

As I analyzed above, it seems that not any case can cause this issues. Can you find the repro step?

[marius-bughiu]

I don't have any steps to reproduce it unfortunately.

I have two reports of it happening (besides the telemetry), and the only 'clue' that we have is that it typically happens after locking/unlocking the PC or RDP session.

I would need to dig a bit more into the code to better understand what is possible, but the only thing I can think of that would be able to cause this is if everything runs on the same thread (dispatcher), making the lock re-entrant, and then a WM_TABLET_DELETED or WM_TABLET_ADDED gets processed while a UnRegisterHwndForInput or RegisterHwndForInput is already in progress. Then it no longer matters that RefreshTablets() is called from within the lock, being the same thread it will do nothing to protect against concurrency.

I will try and see if I can create a POC to force-reproduce it.