Input/Stylus: Make WISP pen processing more resilient against disposal

[etvorun]

Summary

Fixes a hang in the WPF WISP stylus stack where calling PenThreadWorker.WorkerGetTabletsInfo() on an already-disposed worker causes the calling thread to block indefinitely, preventing the application process from exiting cleanly.

A secondary fix stops PenThreadPool from returning disposed PenThread objects as valid candidates.

What changed

PenThreadWorker.cs

• Added internal bool IsDisposed => __disposed; property.
• Added a disposed guard at the start of WorkerGetTabletsInfo() — returns Array.Empty<TabletDeviceInfo>() immediately when disposed, consistent with all other Worker* methods (WorkerAddPenContext, WorkerRemovePenContext, WorkerCreateContext, WorkerAcquireTabletLocks, etc.).

PenThread.cs

• Added internal bool IsDisposed => _penThreadWorker.IsDisposed; property forwarding to the worker.
• Made _penThreadWorker field readonly (assigned in constructor only, never reassigned).
• Removed the now-redundant null check in DisposeHelper (field is readonly and always set).

PenThreadPool.cs

• Extended the existing WeakReference cleanup loop to also prune and skip PenThread entries that are disposed but still strongly referenced (e.g., kept alive via the _handles array).

Why

WorkerGetTabletsInfo was the only Worker* method without a disposed guard. When the pen thread's ThreadProc has already exited (because __disposed is true), it will never call DoneEvent.Set(). Any caller of WorkerGetTabletsInfo() hitting the disposed worker blocks on WaitOne() forever.

PenThreadPool previously pruned only dead WeakReference targets. A disposed-but-still-alive PenThread (kept alive by its registered PenContext handles) passed the old aliveness check and was returned as a valid, ready-to-use thread — compounding the above hang.

Validation notes

• Aligned with the previously validated internal source fix behavior.
• Manual validation: regression tested against the reported shutdown hang scenario.

Fixes #11486

Microsoft Reviewers: Open in CodeFlow

[Copilot]

Pull request overview

Improves shutdown robustness in the WPF WISP stylus stack by preventing synchronous pen-thread operations from hanging when invoked after (or during) disposal, and by avoiding selection of disposed pen threads from the pool.

Changes:

• Add IsDisposed surfaces on PenThreadWorker and PenThread, and add a disposed guard in PenThreadWorker.WorkerGetTabletsInfo() to return an empty array instead of blocking.
• Update PenThreadPool candidate selection to prune dead/disposed PenThread entries and avoid returning disposed threads.
• Make PenThread._penThreadWorker readonly and remove the redundant null-conditional dispose.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File	Description
src/Microsoft.DotNet.Wpf/src/PresentationCore/System/Windows/Input/Stylus/Wisp/PenThreadWorker.cs	Exposes disposal state and adds an early-return guard in WorkerGetTabletsInfo() to avoid hangs after disposal.
src/Microsoft.DotNet.Wpf/src/PresentationCore/System/Windows/Input/Stylus/Wisp/PenThread.cs	Forwards IsDisposed and tightens disposal semantics by making the worker field readonly.
src/Microsoft.DotNet.Wpf/src/PresentationCore/System/Windows/Input/Stylus/Wisp/PenThreadPool.cs	Prunes disposed pen threads from the pool so they aren’t selected as viable candidates.

Comments suppressed due to low confidence (2)

src/Microsoft.DotNet.Wpf/src/PresentationCore/System/Windows/Input/Stylus/Wisp/PenThreadWorker.cs:608

• PR description states WorkerGetTabletsInfo is now guarded 'consistent with' WorkerCreateContext/WorkerAcquireTabletLocks/etc., but those methods currently do not check __disposed and will also block on DoneEvent if called after disposal. Either update the PR description for accuracy or consider applying a similar disposed guard pattern to those synchronous Worker* methods as well.

        internal bool IsDisposed => __disposed;

        internal TabletDeviceInfo[] WorkerGetTabletsInfo()
        {
            if (__disposed)
            {
                return Array.Empty<TabletDeviceInfo>();
            }

            // Set data up for this call
            WorkerOperationGetTabletsInfo getTablets = new WorkerOperationGetTabletsInfo();
            
            lock(_workerOperationLock)
            {
                _workerOperation.Add(getTablets);
            }

            // Kick thread to do this work.
            MS.Win32.Penimc.UnsafeNativeMethods.RaiseResetEvent(_pimcResetHandle);

            // Wait for this work to be completed.
            getTablets.DoneEvent.WaitOne();
            getTablets.DoneEvent.Close();
        
            return getTablets.TabletDevicesInfo;
        }


        internal PenContextInfo WorkerCreateContext(IntPtr hwnd, IPimcTablet3 pimcTablet)
        {
            WorkerOperationCreateContext createContextOperation = new WorkerOperationCreateContext(
                                                                    hwnd,
                                                                    pimcTablet);
            lock(_workerOperationLock)
            {
                _workerOperation.Add(createContextOperation);
            }

            // Kick thread to do this work.
            MS.Win32.Penimc.UnsafeNativeMethods.RaiseResetEvent(_pimcResetHandle);

            // Wait for this work to be completed.
            createContextOperation.DoneEvent.WaitOne();
            createContextOperation.DoneEvent.Close();

            return createContextOperation.Result;
        }

src/Microsoft.DotNet.Wpf/src/PresentationCore/System/Windows/Input/Stylus/Wisp/PenThreadWorker.cs:585

• WorkerGetTabletsInfo still has a potential indefinite wait if Dispose() races with this call: the method can observe __disposed == false, enqueue the operation and then block on DoneEvent, but ThreadProc exits immediately once __disposed becomes true and may never drain _workerOperation to signal the event. Consider making the wait bounded (timeout + safe fallback), or ensuring Dispose/ThreadProc unblocks/purges pending WorkerOperations by setting their DoneEvent(s) before exiting.

        internal TabletDeviceInfo[] WorkerGetTabletsInfo()
        {
            if (__disposed)
            {
                return Array.Empty<TabletDeviceInfo>();
            }

            // Set data up for this call
            WorkerOperationGetTabletsInfo getTablets = new WorkerOperationGetTabletsInfo();
            
            lock(_workerOperationLock)
            {
                _workerOperation.Add(getTablets);
            }

            // Kick thread to do this work.
            MS.Win32.Penimc.UnsafeNativeMethods.RaiseResetEvent(_pimcResetHandle);

            // Wait for this work to be completed.
            getTablets.DoneEvent.WaitOne();
            getTablets.DoneEvent.Close();
        

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.