Fix race on stopping monitor (#1377)

* Fix race on stopping monitor

This PR fixes the race condition when disposing of the RegistryMonitor
and when detecting the stop.

This PR modifies Dispose to call Stop() which will cause the wait
handler to fire and stop the Monitor thread which will properly dispose
of the object.

Author: WardenGnaw

src/DebugEngineHost/RegistryMonitor.cs

@@ -47,14 +47,15 @@ private static extern int RegNotifyChangeKeyValue(SafeRegistryHandle hKey, bool
 
         #endregion
 
-        private HostConfigurationSection _section;
+        private readonly HostConfigurationSection _section;
         private readonly bool _watchSubtree;
 
-        // Set when registry value is changed
-        private AutoResetEvent m_changeEvent;
-
         // Set when monitoring is stopped
-        private AutoResetEvent m_stoppedEvent;
+        private AutoResetEvent _stoppedEvent;
+
+        // Members to handle multiple stop calls.
+        private bool _isStopped = false;
+        private readonly object _stopLock = new object();
 
         /// <summary>
         /// Occurs when the specified registry key has changed.
@@ -80,9 +81,13 @@ public void Start()
 
         public void Stop()
         {
-            if (m_stoppedEvent != null)
+            lock (_stopLock)
             {
-                m_stoppedEvent.Set();
+                if (!_isStopped)
+                {
+                    _stoppedEvent?.Set();
+                    _isStopped = true;
+                }
             }
         }
 
@@ -93,53 +98,52 @@ private void Monitor()
             bool stopped = false;
             try
             {
-                m_stoppedEvent = new AutoResetEvent(false);
-                m_changeEvent = new AutoResetEvent(false);
-
-                IntPtr handle = m_changeEvent.SafeWaitHandle.DangerousGetHandle();
-
-                int errorCode = RegNotifyChangeKeyValue(_section.Handle, _watchSubtree, RegChangeNotifyFilter.REG_NOTIFY_CHANGE_NAME | RegChangeNotifyFilter.REG_NOTIFY_CHANGE_LAST_SET, handle, true);
-                if (errorCode != 0) // 0 is ERROR_SUCCESS
-                {
-                    _nativsLogger?.WriteLine(LogLevel.Error, Resource.Error_WatchRegistry, errorCode);
-                }
-                else
+                _stoppedEvent = new AutoResetEvent(false);
+                using (AutoResetEvent registryChangedEvent = new AutoResetEvent(false))
                 {
-                    while (!stopped)
-                    {
-                        int waitResult = WaitHandle.WaitAny(new WaitHandle[] { m_stoppedEvent, m_changeEvent });
+                    IntPtr handle = registryChangedEvent.SafeWaitHandle.DangerousGetHandle();
 
-                        if (waitResult == 0)
-                        {
-                            stopped = true;
-                        }
-                        else
+                    int errorCode = RegNotifyChangeKeyValue(_section.Handle, _watchSubtree, RegChangeNotifyFilter.REG_NOTIFY_CHANGE_NAME | RegChangeNotifyFilter.REG_NOTIFY_CHANGE_LAST_SET, handle, true);
+                    if (errorCode != 0) // 0 is ERROR_SUCCESS
+                    {
+                        _nativsLogger?.WriteLine(LogLevel.Error, Resource.Error_WatchRegistry, errorCode);
+                    }
+                    else
+                    {
+                        while (!stopped)
                         {
-                            errorCode = RegNotifyChangeKeyValue(_section.Handle, _watchSubtree, RegChangeNotifyFilter.REG_NOTIFY_CHANGE_NAME | RegChangeNotifyFilter.REG_NOTIFY_CHANGE_LAST_SET, handle, true);
-                            if (errorCode != 0) // 0 is ERROR_SUCCESS
+                            int waitResult = WaitHandle.WaitAny(new WaitHandle[] { _stoppedEvent, registryChangedEvent });
+
+                            if (waitResult == 0)
                             {
-                                _nativsLogger?.WriteLine(LogLevel.Error, Resource.Error_WatchRegistry, errorCode);
-                                break;
+                                stopped = true;
+                            }
+                            else
+                            {
+                                errorCode = RegNotifyChangeKeyValue(_section.Handle, _watchSubtree, RegChangeNotifyFilter.REG_NOTIFY_CHANGE_NAME | RegChangeNotifyFilter.REG_NOTIFY_CHANGE_LAST_SET, handle, true);
+                                if (errorCode != 0) // 0 is ERROR_SUCCESS
+                                {
+                                    _nativsLogger?.WriteLine(LogLevel.Error, Resource.Error_WatchRegistry, errorCode);
+                                    break;
+                                }
+                                RegChanged?.Invoke(this, null);
                             }
-                            RegChanged?.Invoke(this, null);
                         }
                     }
                 }
             }
             finally
             {
-                _section.Dispose();
-                m_stoppedEvent?.Dispose();
-                m_changeEvent?.Dispose();
+                _stoppedEvent.Dispose();
+                _stoppedEvent = null;
 
-                m_stoppedEvent = null;
-                m_changeEvent = null;
+                _section.Dispose();
             }
         }
 
         public void Dispose()
         {
-            m_stoppedEvent?.Dispose();
+            Stop(); // Stopping the monitor will dispose of the AutoResetEvent and HostConfigurationSection
         }
     }
 }