Throw IndexOutOfRangeException in WispLogic.CoalesceAndQueueStylusEvent

lindexi commented 5 years ago

.NET Core Version: 3.0.100-preview5-011568
Windows version: 1903 18362.145
Does the bug reproduce also in WPF for .NET Framework 4.8?: Find in 4.7

Problem description:

The application crash.

We find many customs will break the application when he touch the application. I catch the AppDomain.CurrentDomain.UnhandledException and find the WispLogic.CoalesceAndQueueStylusEvent may be break the application.

See: https://stackoverflow.com/questions/46049673/wpf-touch-application-partially-freezes-on-net-framework-4-7

And Bruno V tell me that it fix in .NET 4.7.1, but it seems not fix in dotnet core.

I can not debug it, because 200 users found the crash among 300,000 users.

.NET 4.7: 200 users found the crash among 300,000 users.
.NET Core 3.0: 2 users found the crash among 600 users.

Actual behavior:

exceptions

ExceptionType: System.IndexOutOfRangeException

callstack for crashes

   at System.Collections.Generic.Dictionary`2.Insert(TKey key, TValue value, Boolean add)
   at System.Windows.Input.StylusWisp.WispLogic.CoalesceAndQueueStylusEvent(RawStylusInputReport inputReport)
   at System.Windows.Input.StylusWisp.WispLogic.ProcessInputReport(RawStylusInputReport inputReport)
   at System.Windows.Input.PenContext.FirePackets(Int32 stylusPointerId, Int32[] data, Int32 timestamp)
   at System.Windows.Input.PenThreadWorker.FlushCache(Boolean goingOutOfRange)
   at System.Windows.Input.PenThreadWorker.FireEvent(PenContext penContext, Int32 evt, Int32 stylusPointerId, Int32 cPackets, Int32 cbPacket, IntPtr pPackets)
   at System.Windows.Input.PenThreadWorker.ThreadProc()
   at System.Threading.ThreadHelper.ThreadStart_Context(Object state)
   at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state, Boolean preserveSyncCtx)
   at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state, Boolean preserveSyncCtx)
   at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state)
   at System.Threading.ThreadHelper.ThreadStart()

Expected behavior:

The application can run.

Minimal repro:

An empty WPF application and run in touch device.

lindexi commented 5 years ago

My friend @kkwpsv find the code in .NET 4.7 for Windows 10 Creators Update have not add lock in WispLogic.CoalesceAndQueueStylusEvent. So it seems that it only can find in .NET 4.7 for Windows 10 Creators Update and it fix in dotnet core.

lindexi commented 5 years ago

Why it break in .NET Framework 4.7 for Windows10 Creators Update?

My friend @kkwpsv download the source code from https://referencesource.microsoft.com and find the .NET 4.7 for Windows 10 Creators Update have not add lock to WispLogic.CoalesceAndQueueStylusEvent and the other add the lock.

So it only break in .NET Framework 4.7 for Windows10 Creators Update. And .NET Framework 4.7 RTM add the lock to WispLogic.CoalesceAndQueueStylusEvent and fix it.

Which version add lock?

.NET 4.7 RTM (Please check for your download file)
.NET 4.7.2 RS4
.NET 4.7.2 ZDP
.NET 4.8 RTM

Which version remove lock?

The funny thing is .NET 4.7 RTM add the lock to WispLogic.CoalesceAndQueueStylusEvent and the .NET 4.7.1 Downlevel remove it.

The .NET 4.7.2 for Windows 10 April 2018 Creators Update is the frist version to add lock and it seems fix in .NET 4.7.2 not fix in .NET 4.7.1

See https://stackoverflow.com/a/56575786/6116637

c# - WPF applications stop responding to touches after adding or removing tablet devices - Stack Overflow

weltkante commented 5 years ago

Last time I checked not all downloads at referencesource were correctly labeled. Be careful how you associate them with the actual .NET version, they may actually be another version than the zip file name says.

lindexi commented 5 years ago

@weltkante Could you tell me which it is incorrectly? I want to fix it.

weltkante commented 5 years ago

@lindexi If you downloaded within the last month then DotNet472RS4.zip is actually the same as 4.7 RTM and does not contain 4.7.2 source code. I think that was the only mistake left after I've sent them feedback. If you downloaded before last month then there were more files mixed up, I don't have a complete list, but at least dotnet462RTM.zip, DotNet47RS2.zip and DotNet47RTM.zip downloads were all wrong at some point in the past, but have been corrected since. (Also I do not know what the 'ZDP' release is, so I cannot tell if its wrong or not.)

kkwpsv commented 5 years ago

@lindexi If you downloaded within the last month then DotNet472RS4.zip is actually the same as 4.7 RTM and does not contain 4.7.2 source code. I think that was the only mistake left. If you downloaded before then there were more files mixed up. (I do not know what the 'ZDP' release is, so I cannot tell if its wrong or not.)

I download them from "https://referencesource.microsoft.com/" today. I follow the version that it says.

weltkante commented 5 years ago

Just checked, DotNet472RS4.zip still contains 4.7 RTM and not 4.7.2

lindexi commented 2 years ago

The WispLogic.CoalesceAndQueueStylusEvent code in .NET 4.7 for Windows 10 Creators Update, DotNet47RS2.zip:

        /// <summary>
        /// This function will appropriately coalesce any move messages if needed
        /// and will all appropriately coalesced moves and also any non-move messages.
        /// This ensures both the responsiveness and consistency of the stack.
        /// </summary>
        /// <param name="inputReport">The report to queue</param>
        /// <SecurityNote>
        ///     Critical:  Calls SecurityCritical method QueueStylusEvent
        /// </SecurityNote>
        [SecurityCritical]
        void CoalesceAndQueueStylusEvent(RawStylusInputReport inputReport)
        {
            StylusDeviceBase stylusDevice = inputReport?.StylusDevice?.StylusDeviceImpl;

            // DDVSO:174153
            // Due to changes both in WISP and in the underlying PenIMC code, it is possible that
            // the stylus device here could be null.  If this is the case, the lookups will fail
            // with an exception.
            if (stylusDevice == null)
            {
                return;
            }

            // DevDiv:652804
            // Previously the pen thread would blindly shove any move from Wisp onto the stylus
            // queue.  This is a problem if the main thread stalls but the pen thread does not.
            // Wisp will coalesce data, but only if the pen thread fails to pick up the event.
            // Otherwise, we need to re-implement coalescing of move events here so that we
            // make move data available via GetIntermediateTouchPoints but do not flood the
            // stylus queue with old moves, creating lag in user interaction.  To do that we 
            // detect stalls in the main thread by checking if the last move has processed.
            // If not, we coalesce moves together until we can queue up the coalesced events.

            RawStylusInputReport lastMoveReport = null;
            RawStylusInputReport coalescedMove = null;

            _lastMovesQueued.TryGetValue(stylusDevice, out lastMoveReport);
            _coalescedMoves.TryGetValue(stylusDevice, out coalescedMove);

            // All moves now go through a coalesce to simplify logic
            if (inputReport.Actions == RawStylusActions.Move)
            {
                // Start a new coalescing report if none exists
                if (coalescedMove == null)
                {
                    _coalescedMoves[stylusDevice] = inputReport;
                    coalescedMove = inputReport;
                }
                // Add new move to coalesced
                else
                {
                    // GetRawPacketData creates copies, so only call them once
                    int[] oldData = coalescedMove.GetRawPacketData();
                    int[] newData = inputReport.GetRawPacketData();
                    int[] mergedData = new int[oldData.Length + newData.Length];

                    oldData.CopyTo(mergedData, 0);
                    newData.CopyTo(mergedData, oldData.Length);

                    coalescedMove = new RawStylusInputReport(
                            coalescedMove.Mode,
                            coalescedMove.Timestamp,
                            coalescedMove.InputSource,
                            coalescedMove.PenContext,
                            coalescedMove.Actions,
                            coalescedMove.TabletDeviceId,
                            coalescedMove.StylusDeviceId,
                            mergedData
                            );

                    coalescedMove.StylusDevice = stylusDevice.StylusDevice;

                    _coalescedMoves[stylusDevice] = coalescedMove;
                }

                // We can't queue any move if one is still waiting for processing
                if (lastMoveReport != null
                    && lastMoveReport.IsQueued)
                {
                    return;
                }
            }

            // If we get this far, we are queuing a coalesced move if it exists
            if (coalescedMove != null)
            {
                QueueStylusEvent(coalescedMove);

                // Set last move and cleanup coalescing tracking
                _lastMovesQueued[stylusDevice] = coalescedMove;
                _coalescedMoves.Remove(stylusDevice);
            }

            // Move always queues via coalescing, so only queue here if not a move
            // This has to be done post-coalesced move to maintain order of touch
            // operations
            if (inputReport.Actions != RawStylusActions.Move)
            {
                QueueStylusEvent(inputReport);

                // Once we see a non-move, we should get no more input for this particular chain
                // so we can remove the stored prior moves (if they exist).
                _lastMovesQueued.Remove(stylusDevice);
            }
        }

The code in dotnet 6.0.1:

https://github.com/dotnet/wpf/blob/0350d04790959e8ebdd3ceab0d34a40f811f019d/src/Microsoft.DotNet.Wpf/src/PresentationCore/System/Windows/Input/Stylus/Wisp/WispLogic.cs#L162-L264

dotnet / wpf

Throw IndexOutOfRangeException in WispLogic.CoalesceAndQueueStylusEvent #935

Which version add lock?

Which version remove lock?