Fix WinEventHook to be out of process, installed on the main thread.

Author: govert

Source/ExcelDna.IntelliSense/IntelliSenseServer.cs

@@ -125,7 +125,11 @@ private static void CurrentDomain_ProcessExit(object sender, EventArgs e)
             {
                 // Parachute in asap a call to prevent further XLPenHelper calls
                 XlCall.ShutdownStarted();
-                Deactivate();
+
+                // Don't try to clean up clean up on process exit - all our resources are in-process anyway
+                // Leads to an error with the main thread SynchronizationContext, which might be shut down already.
+                // In particular we don't have to call UnhookWinEvent: "If the client's thread ends, the system automatically calls this function."
+                // Deactivate();
             }
             Logger.Initialization.Verbose("IntelliSenseServer ProcessExit End");
 

Source/ExcelDna.IntelliSense/UIMonitor/FormulaEditWatcher.cs

@@ -68,15 +68,17 @@ public IntPtr FormulaEditWindow
         }
 
         SynchronizationContext _syncContextAuto;
+        SynchronizationContext _syncContextMain;
         WindowWatcher _windowWatcher;
 
         IntPtr            _hwndFormulaBar;
         IntPtr            _hwndInCellEdit;
         FormulaEditFocus  _formulaEditFocus;
 
-        public FormulaEditWatcher(WindowWatcher windowWatcher, SynchronizationContext syncContextAuto)
+        public FormulaEditWatcher(WindowWatcher windowWatcher, SynchronizationContext syncContextAuto, SynchronizationContext syncContextMain)
         {
             _syncContextAuto = syncContextAuto;
+            _syncContextMain = syncContextMain;
             _windowWatcher = windowWatcher;
             _windowWatcher.FormulaBarWindowChanged += _windowWatcher_FormulaBarWindowChanged;
             _windowWatcher.InCellEditWindowChanged += _windowWatcher_InCellEditWindowChanged;
@@ -283,7 +285,7 @@ void InstallLocationMonitor(IntPtr hWnd)
             {
                _windowLocationWatcher.Dispose();
             }
-            _windowLocationWatcher = new WindowLocationWatcher(hWnd, _syncContextAuto);
+            _windowLocationWatcher = new WindowLocationWatcher(hWnd, _syncContextAuto, _syncContextMain);
             _windowLocationWatcher.LocationChanged += _windowLocationWatcher_LocationChanged;
         }
 

Source/ExcelDna.IntelliSense/UIMonitor/PopupListWatcher.cs

@@ -21,12 +21,14 @@ class PopupListWatcher : IDisposable
         public IntPtr PopupListHandle => _hwndPopupList;
 
         SynchronizationContext _syncContextAuto;
+        SynchronizationContext _syncContextMain;
         WindowWatcher _windowWatcher;
         WinEventHook _selectionChangeHook = null;
 
-        public PopupListWatcher(WindowWatcher windowWatcher, SynchronizationContext syncContextAuto)
+        public PopupListWatcher(WindowWatcher windowWatcher, SynchronizationContext syncContextAuto, SynchronizationContext syncContextMain)
         {
             _syncContextAuto = syncContextAuto;
+            _syncContextMain = syncContextMain;
             _windowWatcher = windowWatcher;
             _windowWatcher.PopupListWindowChanged += _windowWatcher_PopupListWindowChanged;
             _windowWatcher.FormulaEditLocationChanged += _windowWatcher_FormulaEditLocationChanged;
@@ -104,7 +106,7 @@ void InstallEventHandlers()
                 // TODO: Clean up 
                 var hwndListView = Win32Helper.GetFirstChildWindow(_hwndPopupList);
 
-                _selectionChangeHook = new WinEventHook(WinEventHook.WinEvent.EVENT_OBJECT_SELECTION, WinEventHook.WinEvent.EVENT_OBJECT_SELECTION, _syncContextAuto, hwndListView);
+                _selectionChangeHook = new WinEventHook(WinEventHook.WinEvent.EVENT_OBJECT_SELECTION, WinEventHook.WinEvent.EVENT_OBJECT_SELECTION, _syncContextAuto, _syncContextMain, hwndListView);
                 _selectionChangeHook.WinEventReceived += _selectionChangeHook_WinEventReceived;
                 Logger.WindowWatcher.Verbose($"PopupList selection event handler added");
             }

Source/ExcelDna.IntelliSense/UIMonitor/UIMonitor.cs

@@ -47,9 +47,9 @@ void RunUIAutomation()
             Logger.Monitor.Verbose("UIMonitor.RunUIAutomation installing watchers");
 
             // Create and hook together the various watchers
-            _windowWatcher = new WindowWatcher(_syncContextAuto);
-            _formulaEditWatcher = new FormulaEditWatcher(_windowWatcher, _syncContextAuto);
-            _popupListWatcher = new PopupListWatcher(_windowWatcher, _syncContextAuto);
+            _windowWatcher = new WindowWatcher(_syncContextAuto, _syncContextMain);
+            _formulaEditWatcher = new FormulaEditWatcher(_windowWatcher, _syncContextAuto, _syncContextMain);
+            _popupListWatcher = new PopupListWatcher(_windowWatcher, _syncContextAuto, _syncContextMain);
             _excelToolTipWatcher = new ExcelToolTipWatcher(_windowWatcher, _syncContextAuto);
 
             // These are the events we're interested in for showing, hiding and updating the IntelliSense forms
@@ -279,7 +279,6 @@ public void Dispose()
             _syncContextAuto.Send(delegate
                 {
                     // Remove all event handlers ASAP
-                    // Automation.RemoveAllEventHandlers();
                     if (_windowWatcher != null)
                     {
                         _windowWatcher.Dispose();

Source/ExcelDna.IntelliSense/UIMonitor/WinEvents.cs

@@ -1,12 +1,16 @@
 using System;
 using System.ComponentModel;
+using System.Diagnostics;
 using System.Runtime.InteropServices;
 using System.Threading;
 
 namespace ExcelDna.IntelliSense
 {
     // This class sets up a WinEventHook for the main Excel process - watching for a range or event types specified.
-    // Events received (on the main Excel thread) are posted onto the Automation thread (via syncContextAuto)
+    // Events received (on the main Excel thread) are posted by the handler onto the Automation thread (via syncContextAuto)
+    // NOTE: Currently we make the SetWinEventHook call on the main Excel thread, which is pumping Windows messages.
+    //       In an alternative implementation we could either create our own thread that pumps Windows messages and use the for WinEvents, 
+    //       or we could change our automation thread to also pump windows messages.
     class WinEventHook : IDisposable
     {
         public class WinEventArgs : EventArgs
@@ -106,39 +110,82 @@ public enum WinEventObjectId : int
 
         public event EventHandler<WinEventArgs> WinEventReceived;
 
-        readonly IntPtr _hWinEventHook;
+        /* readonly */ IntPtr _hWinEventHook;
         readonly SynchronizationContext _syncContextAuto;
+        readonly SynchronizationContext _syncContextMain;
         readonly IntPtr _hWndFilterOrZero;    // If non-zero, only these window events are processed
         readonly WinEventDelegate _handleWinEventDelegate;  // Ensures delegate that we pass to SetWinEventHook is not GC'd
+        readonly WinEvent _eventMin;
+        readonly WinEvent _eventMax;
 
-        public WinEventHook(WinEvent eventMin, WinEvent eventMax, SynchronizationContext syncContextAuto, IntPtr hWndFilterOrZero)
+        // Can be called on any thread, but installed by calling in to the main thread, and will only start receiving events then
+        public WinEventHook(WinEvent eventMin, WinEvent eventMax, SynchronizationContext syncContextAuto, SynchronizationContext syncContextMain, IntPtr hWndFilterOrZero)
         {
-            if (syncContextAuto == null)
-                throw new ArgumentNullException(nameof(syncContextAuto));
-            _syncContextAuto = syncContextAuto;
+            _syncContextAuto = syncContextAuto ?? throw new ArgumentNullException(nameof(syncContextAuto));
+            _syncContextMain = syncContextMain ?? throw new ArgumentNullException(nameof(syncContextMain));
             _hWndFilterOrZero = hWndFilterOrZero;
-            var xllModuleHandle = Win32Helper.GetXllModuleHandle();
-            var excelProcessId = Win32Helper.GetExcelProcessId();
             _handleWinEventDelegate = HandleWinEvent;
-            _hWinEventHook = SetWinEventHook(eventMin, eventMax, xllModuleHandle, _handleWinEventDelegate, excelProcessId, 0, SetWinEventHookFlags.WINEVENT_INCONTEXT);
+            _eventMin = eventMin;
+            _eventMax = eventMax;
+            syncContextMain.Post(InstallWinEventHook, null);
+        }
+
+        // Must run on the main Excel thread (or another thread where Windows messages are pumped)
+        void InstallWinEventHook(object _)
+        {
+            var excelProcessId = Win32Helper.GetExcelProcessId();
+            _hWinEventHook = SetWinEventHook(_eventMin, _eventMax, IntPtr.Zero, _handleWinEventDelegate, excelProcessId, 0, SetWinEventHookFlags.WINEVENT_OUTOFCONTEXT);
             if (_hWinEventHook == IntPtr.Zero)
             {
+                Debug.Print("++++++++++++++ SetWinEventHook failed +++++++++++++++++++++++++++");
                 Logger.WinEvents.Error("SetWinEventHook failed");
                 // Is SetLastError used? - SetWinEventHook documentation does not indicate so
                 throw new Win32Exception("SetWinEventHook failed");
             }
+            // Debug.Print($"++++++++++++++ SetWinEventHook Success on thread {Thread.CurrentThread.ManagedThreadId} \r\n {Environment.StackTrace} \r\n +++++++++++++++++++++++++++");
             Logger.WinEvents.Info($"SetWinEventHook success on thread {Thread.CurrentThread.ManagedThreadId}");
         }
 
-        // This runs on the Excel main thread - get off quickly
+        // Must run on the same thread that InstallWinEventHook ran on
+        void UninstallWinEventHook(object _)
+        {
+            if (_hWinEventHook == IntPtr.Zero)
+            {
+                Logger.WinEvents.Warn($"UnhookWinEvent unexpectedly called with no hook installed - thread {Thread.CurrentThread.ManagedThreadId}");
+                return;
+            }
+
+            try
+            {
+                Logger.WinEvents.Info($"UnhookWinEvent called on thread {Thread.CurrentThread.ManagedThreadId}");
+                bool result = UnhookWinEvent(_hWinEventHook);
+                if (!result)
+                {
+                    // GetLastError?
+                    Logger.WinEvents.Info($"UnhookWinEvent failed");
+                }
+                else
+                {
+                    Logger.WinEvents.Info("UnhookWinEvent success");
+                }
+            }
+            catch (Exception ex)
+            {
+                Logger.WinEvents.Warn($"UnhookWinEvent Exception {ex}");
+            }
+            finally
+            {
+                _hWinEventHook = IntPtr.Zero;
+            }
+        }
+
+        // This runs on the Excel main thread (usually, not always) - get off quickly
         void HandleWinEvent(IntPtr hWinEventHook, WinEvent eventType, IntPtr hWnd,
                             WinEventObjectId idObject, int idChild, uint dwEventThread, uint dwmsEventTime)
         {
+            // Debug.Print($"++++++++++++++ WinEvent Received: {eventType} on thread {Thread.CurrentThread.ManagedThreadId} from thread {dwEventThread} +++++++++++++++++++++++++++");
             try
             {
-                if (disposedValue)
-                    return;
-
                 if (_hWndFilterOrZero != IntPtr.Zero && hWnd != _hWndFilterOrZero)
                     return;
 
@@ -154,6 +201,7 @@ void HandleWinEvent(IntPtr hWinEventHook, WinEvent eventType, IntPtr hWnd,
             }
         }
 
+        // A quick filter that runs on the Excel main thread (or other thread handling the WinEvent)
         bool IsSupportedWinEvent(WinEvent winEvent)
         {
             return winEvent == WinEvent.EVENT_OBJECT_CREATE ||
@@ -179,49 +227,10 @@ void OnWinEventReceived(object winEventArgsObj)
         }
 
         #region IDisposable Support
-        bool disposedValue = false; // To detect redundant calls
-
-        protected virtual void Dispose(bool disposing)
-        {
-            if (!disposedValue)
-            {
-                Logger.WinEvents.Info($"WinEventHook Dispose on thread {Thread.CurrentThread.ManagedThreadId}");
-                if (disposing)
-                {
-                    // TODO: dispose managed state (managed objects).
-                }
-                _syncContextAuto.Send(winEventHook =>
-                {
-                    try
-                    {
-                        Logger.WinEvents.Info($"UnhookWinEvent called on thread {Thread.CurrentThread.ManagedThreadId}");
-                        bool result = UnhookWinEvent((IntPtr)winEventHook);
-                        Logger.WinEvents.Info($"UnhookWinEvent success? {result}");
-                    }
-                    catch (Exception ex)
-                    {
-                        Logger.WinEvents.Warn($"UnhookWinEvent Exception {ex}");
-                    }
-                }, _hWinEventHook);
-
-                disposedValue = true;
-            }
-        }
-
-        // TODO: Does this make any sense?
-        //       We _have_to_ Unhook from the automation thread...
-        // ~WinEventHook()
-        //{
-        //   // Do not change this code. Put cleanup code in Dispose(bool disposing) above.
-        //   Dispose(false);
-        // }
-
-        // This code added to correctly implement the disposable pattern.
         public void Dispose()
         {
-            // Do not change this code. Put cleanup code in Dispose(bool disposing) above.
-            Dispose(true);
-            // GC.SuppressFinalize(this);
+            Logger.WinEvents.Info($"WinEventHook Dispose on thread {Thread.CurrentThread.ManagedThreadId}");
+            _syncContextMain.Send(UninstallWinEventHook, null);
         }
         #endregion
     }

Source/ExcelDna.IntelliSense/UIMonitor/WindowLocationWatcher.cs

@@ -15,11 +15,11 @@ public class WindowLocationWatcher : IDisposable
         //       This nearly worked, and meant we were watching many fewer events ...
         //       ...but we missed some of the resizing events for the window, leaving our tooltip stranded.
         //       So until we can find a workaround for that (perhaps a timer would work fine for this), we watch all the LOCATIONCHANGE events.
-        public WindowLocationWatcher(IntPtr hWnd, SynchronizationContext syncContextAuto)
+        public WindowLocationWatcher(IntPtr hWnd, SynchronizationContext syncContextAuto, SynchronizationContext syncContextMain)
         {
             _hWnd = hWnd;
             _syncContextAuto = syncContextAuto;
-            _windowLocationChangeHook = new WinEventHook(WinEventHook.WinEvent.EVENT_OBJECT_LOCATIONCHANGE, WinEventHook.WinEvent.EVENT_OBJECT_LOCATIONCHANGE, _syncContextAuto, _hWnd);
+            _windowLocationChangeHook = new WinEventHook(WinEventHook.WinEvent.EVENT_OBJECT_LOCATIONCHANGE, WinEventHook.WinEvent.EVENT_OBJECT_LOCATIONCHANGE, _syncContextAuto, syncContextMain, _hWnd);
             _windowLocationChangeHook.WinEventReceived += _windowLocationChangeHook_WinEventReceived;
         }
 

Source/ExcelDna.IntelliSense/UIMonitor/WindowWatcher.cs

@@ -124,15 +124,15 @@ internal WindowChangedEventArgs(IntPtr windowHandle, WinEventHook.WinEvent winEv
         public event EventHandler FormulaEditLocationChanged;
 //        public event EventHandler<WindowChangedEventArgs> SelectDataSourceWindowChanged;
 
-        public WindowWatcher(SynchronizationContext syncContextAuto)
+        public WindowWatcher(SynchronizationContext syncContextAuto, SynchronizationContext syncContextMain)
         {
 #pragma warning disable CS0618 // Type or member is obsolete (GetCurrentThreadId) - But for debugging we want to monitor this anyway
             // Debug.Print($"### WindowWatcher created on thread: Managed {Thread.CurrentThread.ManagedThreadId}, Native {AppDomain.GetCurrentThreadId()}");
 #pragma warning restore CS0618 // Type or member is obsolete
 
             // Using WinEvents instead of Automation so that we can watch top-level window changes, but only from the right (current Excel) process.
             // TODO: We need to dramatically reduce the number of events we grab here...
-            _windowStateChangeHook = new WinEventHook(WinEventHook.WinEvent.EVENT_OBJECT_CREATE, WinEventHook.WinEvent.EVENT_OBJECT_LOCATIONCHANGE, syncContextAuto, IntPtr.Zero);
+            _windowStateChangeHook = new WinEventHook(WinEventHook.WinEvent.EVENT_OBJECT_CREATE, WinEventHook.WinEvent.EVENT_OBJECT_LOCATIONCHANGE, syncContextAuto, syncContextMain, IntPtr.Zero);
             // _windowStateChangeHook = new WinEventHook(WinEventHook.WinEvent.EVENT_OBJECT_CREATE, WinEventHook.WinEvent.EVENT_OBJECT_END, syncContextAuto, IntPtr.Zero);
             // _windowStateChangeHook = new WinEventHook(WinEventHook.WinEvent.EVENT_MIN, WinEventHook.WinEvent.EVENT_AIA_END, syncContextAuto, IntPtr.Zero);
 
@@ -146,7 +146,7 @@ public void TryInitialize()
             var focusedWindowHandle = Win32Helper.GetFocusedWindowHandle();
             string className = null;
             if (focusedWindowHandle != IntPtr.Zero)
-                className = Win32Helper.GetClassName(_focusedWindowHandle);
+                className = Win32Helper.GetClassName(focusedWindowHandle);
 
             UpdateFocus(focusedWindowHandle, className);
         }

Source/ExcelDna.IntelliSense/UIMonitor/XlCall.cs

@@ -68,6 +68,8 @@ public static string GetFormulaEditPrefix()
                     return null;
                 }
 
+                // We seem to mis-track in the click case when wPointMode changed from xlModeEnter to xlModeEdit
+
                 Logger.WindowWatcher.Verbose("LPenHelper Status: PointMode: {0}, Formula: {1}, First: {2}, Last: {3}, Caret: {4}",
                     fmlaInfo.wPointMode, Marshal.PtrToStringUni(fmlaInfo.lpch, fmlaInfo.cch), fmlaInfo.ichFirst, fmlaInfo.ichLast, fmlaInfo.ichCaret);