More threading clean-up

(Tooltip move is now broken)

Author: govert

Source/ExcelDna.IntelliSense/IntelliSenseDisplay.cs

@@ -61,11 +61,13 @@ void RunUIAutomation()
         {
             _syncContextAuto = new SingleThreadSynchronizationContext();
 
+            // Create and hook together the various watchers
             _windowWatcher = new WindowWatcher(_syncContextAuto);
             _formulaEditWatcher = new FormulaEditWatcher(_windowWatcher, _syncContextAuto);
             _popupListWatcher = new PopupListWatcher(_windowWatcher, _syncContextAuto);
             // _selectDataSourceWatcher = new SelectDataSourceWatcher(_windowWatcher, _syncContextAuto);
 
+            // These are the events we're interested in for showing, hiding and updating the IntelliSense forms
             _windowWatcher.MainWindowChanged += _windowWatcher_MainWindowChanged;
             _popupListWatcher.SelectedItemChanged += _popupListWatcher_SelectedItemChanged;
             _formulaEditWatcher.StateChanged += _formulaEditWatcher_StateChanged;
@@ -75,30 +77,36 @@ void RunUIAutomation()
             _syncContextAuto.RunOnCurrentThread();
         }
 
-        // Runs on the auto thread
+        #region Receive watcher events, and post to main thread
+        // Runs on our automation thread
         void _windowWatcher_MainWindowChanged(object sender, EventArgs args)
         {
+            Logger.Display.Verbose("! MainWindowChanged");
             _syncContextMain.Post(MainWindowChanged, null);
         }
 
-        // Runs on the auto thread
+        // Runs on our automation thread
         void _popupListWatcher_SelectedItemChanged(object sender, EventArgs args)
         {
+            Logger.Display.Verbose("! PopupList SelectedItemChanged");
             _syncContextMain.Post(PopupListSelectedItemChanged, null);
         }
 
-        // Runs on the auto thread
+        // Runs on our automation thread
         void _formulaEditWatcher_StateChanged(object sender, FormulaEditWatcher.StateChangeEventArgs args)
         {
+            Logger.Display.Verbose($"! FormulaEdit StateChanged ({args.StateChangeType})");
             _syncContextMain.Post(FormulaEditStateChanged, args.StateChangeType);
         }
+        #endregion
 
         // Runs on the main thread
         void MainWindowChanged(object _unused_)
         {
             // TODO: This is to guard against shutdown, but we should not have a race here 
             //       - shutdown should be on the main thread, as is this event handler.
-            if (_windowWatcher == null) return;
+            if (_windowWatcher == null)
+                return;
 
             // TODO: !!! Reset / re-parent ToolTipWindows
             Debug.Print($"IntelliSenseDisplay - MainWindowChanged - New window - {_windowWatcher.MainWindow:X}, Thread {Thread.CurrentThread.ManagedThreadId}");
@@ -128,7 +136,8 @@ void PopupListSelectedItemChanged(object _unused_)
         {
             Debug.Print($"IntelliSenseDisplay - PopupListSelectedItemChanged - New text - {_popupListWatcher?.SelectedItemText}, Thread {Thread.CurrentThread.ManagedThreadId}");
 
-            if (_popupListWatcher == null) return;
+            if (_popupListWatcher == null)
+                return;
             string functionName = _popupListWatcher.SelectedItemText;
 
             IntelliSenseFunctionInfo functionInfo;
@@ -162,7 +171,8 @@ void FormulaEditStateChanged(object stateChangeTypeObj)
             var stateChangeType = (FormulaEditWatcher.StateChangeType)stateChangeTypeObj;
             // Check for watcher already disposed 
             // CONSIDER: How to manage threading with disposal...?
-            if (_formulaEditWatcher == null) return;
+            if (_formulaEditWatcher == null)
+                return;
 
             if (stateChangeType == FormulaEditWatcher.StateChangeType.Move && _argumentsToolTip != null)
             {
@@ -305,7 +315,8 @@ public void Dispose()
             if (_syncContextAuto == null)
                 return;
 
-            _syncContextAuto.Send(delegate
+            // Send is not supported on _syncContextAuto
+            _syncContextAuto.Post(delegate
                 {
                     if (_windowWatcher != null)
                     {
@@ -325,6 +336,8 @@ public void Dispose()
                         _popupListWatcher.Dispose();
                         _popupListWatcher = null;
                     }
+                    _syncContextAuto.Complete();
+                    _syncContextAuto = null;
                 }, null);
 
             _syncContextMain.Send(delegate
@@ -341,10 +354,6 @@ public void Dispose()
                     }
                 }, null);
 
-            _syncContextAuto.Complete();
-            // CONSIDER: Maybe wait for the _syncContextAuto to finish...?
-            _syncContextAuto = null;
-
             _syncContextMain = null;
         }
 

Source/ExcelDna.IntelliSense/Logging.cs

@@ -51,12 +51,14 @@ namespace ExcelDna.IntelliSense
 
     // NOTE: To simplify configuration (so that we provide one TraceSource per referenced assembly) and still allow some grouping
     //       we use the EventId to define a trace event classification.
+    // These names are not surfaces to the Trace Listeners - just the IDs, so we should document them.
     enum IntelliSenseTraceEventId
     {
         Initialization = 1,
         Provider = 2,
         WinEvents = 3,
-        WindowWatcher = 4
+        WindowWatcher = 4,
+        Display = 5
     }
 
     // TraceLogger manages the IntelliSenseTraceSource that we use for logging.
@@ -211,5 +213,6 @@ public void Error(Exception ex, string message, params object[] args)
         static internal Logger Provider { get; } = new Logger(IntelliSenseTraceEventId.Provider);
         static internal Logger WinEvents { get; } = new Logger(IntelliSenseTraceEventId.WinEvents);
         static internal Logger WindowWatcher { get; } = new Logger(IntelliSenseTraceEventId.WindowWatcher);
+        static internal Logger Display { get; } = new Logger(IntelliSenseTraceEventId.Display);
     }
 }

Source/ExcelDna.IntelliSense/SingleThreadSynchronizationContext.cs

@@ -37,7 +37,16 @@ public override void Send(SendOrPostCallback d, object state)
         public void RunOnCurrentThread()
         {
             foreach (var workItem in m_queue.GetConsumingEnumerable())
-                workItem.Key(workItem.Value);
+            {
+                try
+                {
+                    workItem.Key(workItem.Value);
+                }
+                catch (Exception ex)
+                {
+                    Debug.Print($"### Unhandled exception on Automation thread - {ex.Message}");
+                }
+            }
 
             Debug.Print("SingleThreadSynchronizationContext Complete!");
         }

Source/ExcelDna.IntelliSense/ToolTipForm.cs

@@ -14,13 +14,13 @@ class ToolTipForm  : Form
         private Label label;
         Win32Window _owner;
 
-        public ToolTipForm(IntPtr hwndParent)
+        public ToolTipForm(IntPtr hwndOwner)
         {
             InitializeComponent();
-            _owner = new Win32Window(hwndParent);
+            _owner = new Win32Window(hwndOwner);
             // _owner = new NativeWindow();
             // _owner.AssignHandle(hwndParent); (...with ReleaseHandle in Dispose)
-            Debug.Print("Created ToolTipForm with parent " + hwndParent + " Owner Handle: " + (_owner == null ? "<NULL> " : _owner.Handle.ToString()) );
+            Debug.Print($"Created ToolTipForm with owner {hwndOwner}");
         }
 
         public void ShowToolTip()
@@ -89,6 +89,7 @@ protected override CreateParams CreateParams
         {
             get
             {
+                // NOTE: I've seen exception with invalid handle in the base.CreateParams call here...
                 CreateParams baseParams = base.CreateParams;
                 baseParams.ClassStyle |= CS_DROPSHADOW;
                 baseParams.ExStyle |= ( WS_EX_NOACTIVATE | WS_EX_TOOLWINDOW );
@@ -128,7 +129,8 @@ protected override void OnPaint(PaintEventArgs e)
                             // TODO: Empty strings are a problem....
                             var text = run.Text == "" ? " " : run.Text;
 
-                            DrawString(e.Graphics, SystemBrushes.WindowText, ref layoutRect, out textSize, format, text, font);
+                            // TODO: Find the color SystemBrushes.ControlDarkDark
+                            DrawString(e.Graphics, SystemBrushes.WindowFrame, ref layoutRect, out textSize, format, text, font);
                             totalWidth += textSize.Width;
                             lineHeight = Math.Max(lineHeight, textSize.Height);
                         }