From 500cea0fc41e20dcca41250ada8934517e188962 Mon Sep 17 00:00:00 2001 From: Robert LaDuca Date: Tue, 14 Apr 2020 20:16:06 -0700 Subject: [PATCH 1/5] Ensuring COM activation contexts for PenIMC are created per PenThread and cleaned up when the PenThread dies. --- .../src/PenImc/dll/SxSCOMRegistration.cpp | 3 - .../MS/Win32/UnsafeNativeMethodsPenimc.cs | 75 +++++++++++++++---- .../Input/Stylus/Wisp/PenThreadWorker.cs | 3 + 3 files changed, 63 insertions(+), 18 deletions(-) diff --git a/src/Microsoft.DotNet.Wpf/src/PenImc/dll/SxSCOMRegistration.cpp b/src/Microsoft.DotNet.Wpf/src/PenImc/dll/SxSCOMRegistration.cpp index 0e97f1dded4..a51a5b2f138 100644 --- a/src/Microsoft.DotNet.Wpf/src/PenImc/dll/SxSCOMRegistration.cpp +++ b/src/Microsoft.DotNet.Wpf/src/PenImc/dll/SxSCOMRegistration.cpp @@ -46,6 +46,3 @@ extern "C" ULONG_PTR WINAPI RegisterDllForSxSCOM() // Return the context cookie : caller is responsible for deactivating the context. return activationContextCookie; } - - - diff --git a/src/Microsoft.DotNet.Wpf/src/PresentationCore/MS/Win32/UnsafeNativeMethodsPenimc.cs b/src/Microsoft.DotNet.Wpf/src/PresentationCore/MS/Win32/UnsafeNativeMethodsPenimc.cs index 6d055aab318..35931568b94 100644 --- a/src/Microsoft.DotNet.Wpf/src/PresentationCore/MS/Win32/UnsafeNativeMethodsPenimc.cs +++ b/src/Microsoft.DotNet.Wpf/src/PresentationCore/MS/Win32/UnsafeNativeMethodsPenimc.cs @@ -75,6 +75,12 @@ internal static class UnsafeNativeMethods [ThreadStatic] private static IPimcManager3 _pimcManagerThreadStatic; + /// + /// The cookie for the PenIMC activation context. + /// + [ThreadStatic] + private static IntPtr _pimcActCtxCookie = IntPtr.Zero; + #endregion #region PenIMC @@ -87,25 +93,40 @@ internal static class UnsafeNativeMethods /// /// Make sure we load penimc.dll from WPF's installed location to avoid two instances of it. /// - static UnsafeNativeMethods() + private static void EnsurePenImcClassesActivated() { - // Register PenIMC for SxS COM for the lifetime of the process. No need to store the - // cookie; PenIMC's ActivationContext will never be removed from the ActivationContext - // stack. - // - // RegisterDllForSxSCOM returns a non-zero ActivationContextCookie if SxS registration - // succeeds, or IntPtr.Zero if SxS registration fails. - if (IntPtr.Zero == RegisterDllForSxSCOM()) + if (_pimcActCtxCookie == IntPtr.Zero) { - throw new InvalidOperationException(SR.Get(SRID.PenImcSxSRegistrationFailed, ExternDll.Penimc)); - } + // Register PenIMC for SxS COM for the lifetime of the thread. + // + // RegisterDllForSxSCOM returns a non-zero ActivationContextCookie if SxS registration + // succeeds, or IntPtr.Zero if SxS registration fails. + if ((_pimcActCtxCookie = RegisterDllForSxSCOM()) == IntPtr.Zero) + { + throw new InvalidOperationException(SR.Get(SRID.PenImcSxSRegistrationFailed, ExternDll.Penimc)); + } - // Ensure PenIMC loaded from the correct location. - var uncheckedDlls = WpfDllVerifier.VerifyWpfDllSet(ExternDll.Penimc); + // Ensure PenIMC loaded from the correct location. + var uncheckedDlls = WpfDllVerifier.VerifyWpfDllSet(ExternDll.Penimc); - if (uncheckedDlls.Contains(ExternDll.Penimc)) + if (uncheckedDlls.Contains(ExternDll.Penimc)) + { + throw new DllNotFoundException(SR.Get(SRID.PenImcDllVerificationFailed, ExternDll.Penimc)); + } + } + } + + /// + /// Deactivates the activation context for PenIMC objects. + /// + internal static void DeactivatePenImcClasses() + { + if (_pimcActCtxCookie != IntPtr.Zero) { - throw new DllNotFoundException(SR.Get(SRID.PenImcDllVerificationFailed, ExternDll.Penimc)); + if(DeactivateActCtx(0, _pimcActCtxCookie)) + { + _pimcActCtxCookie = IntPtr.Zero; + } } } @@ -129,6 +150,8 @@ internal static IPimcManager3 PimcManager /// private static IPimcManager3 CreatePimcManager() { + EnsurePenImcClassesActivated(); + // Instantiating PimcManager using "new PimcManager()" results // in calling CoCreateInstanceForApp from an immersive process // (like designer). Such a call would fail because PimcManager is not @@ -157,6 +180,8 @@ internal static void CheckedLockWispObjectFromGit(UInt32 gitKey) { if (OSVersionHelper.IsOsWindows8OrGreater) { + EnsurePenImcClassesActivated(); + if (!LockWispObjectFromGit(gitKey)) { throw new InvalidOperationException(); @@ -173,6 +198,8 @@ internal static void CheckedUnlockWispObjectFromGit(UInt32 gitKey) { if (OSVersionHelper.IsOsWindows8OrGreater) { + EnsurePenImcClassesActivated(); + if (!UnlockWispObjectFromGit(gitKey)) { throw new InvalidOperationException(); @@ -192,6 +219,7 @@ internal static void CheckedUnlockWispObjectFromGit(UInt32 gitKey) /// The manager to release the lock for.' private static void ReleaseManagerExternalLockImpl(IPimcManager3 manager) { + EnsurePenImcClassesActivated(); IPimcTablet3 unused = null; manager.GetTablet(ReleaseManagerExt, out unused); } @@ -205,6 +233,7 @@ internal static void ReleaseManagerExternalLock() { if (_pimcManagerThreadStatic != null) { + EnsurePenImcClassesActivated(); ReleaseManagerExternalLockImpl(_pimcManagerThreadStatic); } } @@ -215,6 +244,7 @@ internal static void ReleaseManagerExternalLock() /// The tablet to call through internal static void SetWispManagerKey(IPimcTablet3 tablet) { + EnsurePenImcClassesActivated(); UInt32 latestKey = QueryWispKeyFromTablet(GetWispManagerKey, tablet); // Assert here to ensure that every call through to this specific manager has the same @@ -233,6 +263,7 @@ internal static void LockWispManager() { if (!_wispManagerLocked && _wispManagerKey.HasValue) { + EnsurePenImcClassesActivated(); CheckedLockWispObjectFromGit(_wispManagerKey.Value); _wispManagerLocked = true; } @@ -245,6 +276,7 @@ internal static void UnlockWispManager() { if (_wispManagerLocked && _wispManagerKey.HasValue) { + EnsurePenImcClassesActivated(); CheckedUnlockWispObjectFromGit(_wispManagerKey.Value); _wispManagerLocked = false; } @@ -263,6 +295,8 @@ internal static void AcquireTabletExternalLock(IPimcTablet3 tablet) { int unused = 0; + EnsurePenImcClassesActivated(); + // Call through with special param to release the external lock on the tablet. tablet.GetCursorButtonCount(LockTabletExt, out unused); } @@ -276,6 +310,8 @@ internal static void ReleaseTabletExternalLock(IPimcTablet3 tablet) { int unused = 0; + EnsurePenImcClassesActivated(); + // Call through with special param to release the external lock on the tablet. tablet.GetCursorButtonCount(ReleaseTabletExt, out unused); } @@ -290,6 +326,8 @@ private static UInt32 QueryWispKeyFromTablet(int keyType, IPimcTablet3 tablet) { int key = 0; + EnsurePenImcClassesActivated(); + tablet.GetCursorButtonCount(keyType, out key); if(key == 0) @@ -307,6 +345,8 @@ private static UInt32 QueryWispKeyFromTablet(int keyType, IPimcTablet3 tablet) /// The GIT key for the WISP Tablet internal static UInt32 QueryWispTabletKey(IPimcTablet3 tablet) { + EnsurePenImcClassesActivated(); + return QueryWispKeyFromTablet(GetWispTabletKey, tablet); } @@ -586,7 +626,8 @@ internal static extern bool GetLastSystemEventData( /// Context in which the newly created object will run /// Identifier of the Interface /// Returns the COM object created by CoCreateInstance - [return: MarshalAs(UnmanagedType.Interface)][DllImport(ExternDll.Ole32, ExactSpelling=true, PreserveSig=false)] + [return: MarshalAs(UnmanagedType.Interface)] + [DllImport(ExternDll.Ole32, ExactSpelling=true, PreserveSig=false)] private static extern object CoCreateInstance( [In] ref Guid clsid, @@ -595,6 +636,10 @@ private static extern object CoCreateInstance( int context, [In] ref Guid iid); + + [return: MarshalAs(UnmanagedType.Bool)] + [DllImport(ExternDll.Kernel32, ExactSpelling = true, PreserveSig = false)] + private static extern bool DeactivateActCtx(int flags, IntPtr activationCtxCookie); } #if OLD_ISF diff --git a/src/Microsoft.DotNet.Wpf/src/PresentationCore/System/Windows/Input/Stylus/Wisp/PenThreadWorker.cs b/src/Microsoft.DotNet.Wpf/src/PresentationCore/System/Windows/Input/Stylus/Wisp/PenThreadWorker.cs index a00348b57f1..114a2381c1e 100644 --- a/src/Microsoft.DotNet.Wpf/src/PresentationCore/System/Windows/Input/Stylus/Wisp/PenThreadWorker.cs +++ b/src/Microsoft.DotNet.Wpf/src/PresentationCore/System/Windows/Input/Stylus/Wisp/PenThreadWorker.cs @@ -1289,6 +1289,9 @@ internal void ThreadProc() _pimcContexts[i].ShutdownComm(); } + // Ensure that any activation contexts used on this thread are cleaned. + MS.Win32.Penimc.UnsafeNativeMethods.DeactivatePenImcClasses(); + // Make sure the _pimcResetHandle is still valid after Dispose is called and before // our thread exits. GC.KeepAlive(this); From 25ad4647070b9ec987b6cb9c6095a257f84737e6 Mon Sep 17 00:00:00 2001 From: Robert LaDuca Date: Wed, 15 Apr 2020 08:07:09 -0700 Subject: [PATCH 2/5] Moving creation of COM activation context to the PenThread. --- .../MS/Win32/UnsafeNativeMethodsPenimc.cs | 21 +------------------ .../Input/Stylus/Wisp/PenThreadWorker.cs | 8 ++++++- 2 files changed, 8 insertions(+), 21 deletions(-) diff --git a/src/Microsoft.DotNet.Wpf/src/PresentationCore/MS/Win32/UnsafeNativeMethodsPenimc.cs b/src/Microsoft.DotNet.Wpf/src/PresentationCore/MS/Win32/UnsafeNativeMethodsPenimc.cs index 35931568b94..6bc631c8806 100644 --- a/src/Microsoft.DotNet.Wpf/src/PresentationCore/MS/Win32/UnsafeNativeMethodsPenimc.cs +++ b/src/Microsoft.DotNet.Wpf/src/PresentationCore/MS/Win32/UnsafeNativeMethodsPenimc.cs @@ -93,7 +93,7 @@ internal static class UnsafeNativeMethods /// /// Make sure we load penimc.dll from WPF's installed location to avoid two instances of it. /// - private static void EnsurePenImcClassesActivated() + internal static void EnsurePenImcClassesActivated() { if (_pimcActCtxCookie == IntPtr.Zero) { @@ -150,8 +150,6 @@ internal static IPimcManager3 PimcManager /// private static IPimcManager3 CreatePimcManager() { - EnsurePenImcClassesActivated(); - // Instantiating PimcManager using "new PimcManager()" results // in calling CoCreateInstanceForApp from an immersive process // (like designer). Such a call would fail because PimcManager is not @@ -180,8 +178,6 @@ internal static void CheckedLockWispObjectFromGit(UInt32 gitKey) { if (OSVersionHelper.IsOsWindows8OrGreater) { - EnsurePenImcClassesActivated(); - if (!LockWispObjectFromGit(gitKey)) { throw new InvalidOperationException(); @@ -198,8 +194,6 @@ internal static void CheckedUnlockWispObjectFromGit(UInt32 gitKey) { if (OSVersionHelper.IsOsWindows8OrGreater) { - EnsurePenImcClassesActivated(); - if (!UnlockWispObjectFromGit(gitKey)) { throw new InvalidOperationException(); @@ -219,7 +213,6 @@ internal static void CheckedUnlockWispObjectFromGit(UInt32 gitKey) /// The manager to release the lock for.' private static void ReleaseManagerExternalLockImpl(IPimcManager3 manager) { - EnsurePenImcClassesActivated(); IPimcTablet3 unused = null; manager.GetTablet(ReleaseManagerExt, out unused); } @@ -233,7 +226,6 @@ internal static void ReleaseManagerExternalLock() { if (_pimcManagerThreadStatic != null) { - EnsurePenImcClassesActivated(); ReleaseManagerExternalLockImpl(_pimcManagerThreadStatic); } } @@ -244,7 +236,6 @@ internal static void ReleaseManagerExternalLock() /// The tablet to call through internal static void SetWispManagerKey(IPimcTablet3 tablet) { - EnsurePenImcClassesActivated(); UInt32 latestKey = QueryWispKeyFromTablet(GetWispManagerKey, tablet); // Assert here to ensure that every call through to this specific manager has the same @@ -263,7 +254,6 @@ internal static void LockWispManager() { if (!_wispManagerLocked && _wispManagerKey.HasValue) { - EnsurePenImcClassesActivated(); CheckedLockWispObjectFromGit(_wispManagerKey.Value); _wispManagerLocked = true; } @@ -276,7 +266,6 @@ internal static void UnlockWispManager() { if (_wispManagerLocked && _wispManagerKey.HasValue) { - EnsurePenImcClassesActivated(); CheckedUnlockWispObjectFromGit(_wispManagerKey.Value); _wispManagerLocked = false; } @@ -295,8 +284,6 @@ internal static void AcquireTabletExternalLock(IPimcTablet3 tablet) { int unused = 0; - EnsurePenImcClassesActivated(); - // Call through with special param to release the external lock on the tablet. tablet.GetCursorButtonCount(LockTabletExt, out unused); } @@ -310,8 +297,6 @@ internal static void ReleaseTabletExternalLock(IPimcTablet3 tablet) { int unused = 0; - EnsurePenImcClassesActivated(); - // Call through with special param to release the external lock on the tablet. tablet.GetCursorButtonCount(ReleaseTabletExt, out unused); } @@ -326,8 +311,6 @@ private static UInt32 QueryWispKeyFromTablet(int keyType, IPimcTablet3 tablet) { int key = 0; - EnsurePenImcClassesActivated(); - tablet.GetCursorButtonCount(keyType, out key); if(key == 0) @@ -345,8 +328,6 @@ private static UInt32 QueryWispKeyFromTablet(int keyType, IPimcTablet3 tablet) /// The GIT key for the WISP Tablet internal static UInt32 QueryWispTabletKey(IPimcTablet3 tablet) { - EnsurePenImcClassesActivated(); - return QueryWispKeyFromTablet(GetWispTabletKey, tablet); } diff --git a/src/Microsoft.DotNet.Wpf/src/PresentationCore/System/Windows/Input/Stylus/Wisp/PenThreadWorker.cs b/src/Microsoft.DotNet.Wpf/src/PresentationCore/System/Windows/Input/Stylus/Wisp/PenThreadWorker.cs index 114a2381c1e..843645de5fb 100644 --- a/src/Microsoft.DotNet.Wpf/src/PresentationCore/System/Windows/Input/Stylus/Wisp/PenThreadWorker.cs +++ b/src/Microsoft.DotNet.Wpf/src/PresentationCore/System/Windows/Input/Stylus/Wisp/PenThreadWorker.cs @@ -1153,6 +1153,7 @@ internal void ThreadProc() { Thread.CurrentThread.Name = "Stylus Input"; + try { // @@ -1164,7 +1165,12 @@ internal void ThreadProc() Debug.WriteLine(String.Format("PenThreadWorker::ThreadProc(): Update __penContextWeakRefList loop")); #endif - WorkerOperation [] workerOps = null; + // We need to ensure that the PenIMC COM objects can be used from this thread. + // Try this every outer loop since we're, generally, about to do management + // operations. + MS.Win32.Penimc.UnsafeNativeMethods.EnsurePenImcClassesActivated(); + + WorkerOperation[] workerOps = null; lock(_workerOperationLock) { From 1039f880cf9fccc843c02d84b9e26c762743e377 Mon Sep 17 00:00:00 2001 From: Robert LaDuca Date: Wed, 15 Apr 2020 08:22:54 -0700 Subject: [PATCH 3/5] Removing extraneous line --- .../System/Windows/Input/Stylus/Wisp/PenThreadWorker.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Microsoft.DotNet.Wpf/src/PresentationCore/System/Windows/Input/Stylus/Wisp/PenThreadWorker.cs b/src/Microsoft.DotNet.Wpf/src/PresentationCore/System/Windows/Input/Stylus/Wisp/PenThreadWorker.cs index 843645de5fb..7f18965a234 100644 --- a/src/Microsoft.DotNet.Wpf/src/PresentationCore/System/Windows/Input/Stylus/Wisp/PenThreadWorker.cs +++ b/src/Microsoft.DotNet.Wpf/src/PresentationCore/System/Windows/Input/Stylus/Wisp/PenThreadWorker.cs @@ -1153,7 +1153,6 @@ internal void ThreadProc() { Thread.CurrentThread.Name = "Stylus Input"; - try { // From a7b3096f6b7ec6f203c1ab498e464d7e1076cda8 Mon Sep 17 00:00:00 2001 From: Robert LaDuca Date: Wed, 15 Apr 2020 08:27:06 -0700 Subject: [PATCH 4/5] Comments, etc --- .../MS/Win32/UnsafeNativeMethodsPenimc.cs | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/Microsoft.DotNet.Wpf/src/PresentationCore/MS/Win32/UnsafeNativeMethodsPenimc.cs b/src/Microsoft.DotNet.Wpf/src/PresentationCore/MS/Win32/UnsafeNativeMethodsPenimc.cs index 6bc631c8806..14a88f3ec07 100644 --- a/src/Microsoft.DotNet.Wpf/src/PresentationCore/MS/Win32/UnsafeNativeMethodsPenimc.cs +++ b/src/Microsoft.DotNet.Wpf/src/PresentationCore/MS/Win32/UnsafeNativeMethodsPenimc.cs @@ -608,7 +608,7 @@ internal static extern bool GetLastSystemEventData( /// Identifier of the Interface /// Returns the COM object created by CoCreateInstance [return: MarshalAs(UnmanagedType.Interface)] - [DllImport(ExternDll.Ole32, ExactSpelling=true, PreserveSig=false)] + [DllImport(ExternDll.Ole32, ExactSpelling = true, PreserveSig = false)] private static extern object CoCreateInstance( [In] ref Guid clsid, @@ -618,6 +618,16 @@ private static extern object CoCreateInstance( [In] ref Guid iid); + /// + /// Deactivates the specified Activation Context. + /// + /// + /// See: https://docs.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-deactivateactctx + /// + /// Flags that indicate how the deactivation is to occur. + /// The ULONG_PTR that was passed into the call to ActivateActCtx. + /// This value is used as a cookie to identify a specific activated activation context. + /// True on success, false otherwise. [return: MarshalAs(UnmanagedType.Bool)] [DllImport(ExternDll.Kernel32, ExactSpelling = true, PreserveSig = false)] private static extern bool DeactivateActCtx(int flags, IntPtr activationCtxCookie); From 97fd2634424d41e064515c3bf438a83926c86e7c Mon Sep 17 00:00:00 2001 From: Robert LaDuca Date: Wed, 15 Apr 2020 08:31:31 -0700 Subject: [PATCH 5/5] More comments --- .../MS/Win32/UnsafeNativeMethodsPenimc.cs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/Microsoft.DotNet.Wpf/src/PresentationCore/MS/Win32/UnsafeNativeMethodsPenimc.cs b/src/Microsoft.DotNet.Wpf/src/PresentationCore/MS/Win32/UnsafeNativeMethodsPenimc.cs index 14a88f3ec07..ad2fe90d71a 100644 --- a/src/Microsoft.DotNet.Wpf/src/PresentationCore/MS/Win32/UnsafeNativeMethodsPenimc.cs +++ b/src/Microsoft.DotNet.Wpf/src/PresentationCore/MS/Win32/UnsafeNativeMethodsPenimc.cs @@ -92,7 +92,16 @@ internal static class UnsafeNativeMethods /// /// Make sure we load penimc.dll from WPF's installed location to avoid two instances of it. + /// + /// Add an activation context to the thread's stack to ensure the registration-free COM objects + /// are available. /// + /// + /// PenIMC COM objects are only directly used (functions called on their interfaces) from inside the + /// PenThread. As such, the PenThreads need to create the activation context. The various Dispatcher + /// threads need not do so as they merely pass the RCWs around in manged objects and will only use + /// them via operations queued on their associated PenThread. + /// internal static void EnsurePenImcClassesActivated() { if (_pimcActCtxCookie == IntPtr.Zero)