Fix: subscribe to ItemsChanged before fetching items to avoid missing early events

Agent-Logs-Url: https://github.com/microsoft/PowerToys/sessions/3f380bd3-a56a-4708-80ef-3e7d534c4f6f

Co-authored-by: MuyuanMS <116717757+MuyuanMS@users.noreply.github.com>

src/modules/ZoomIt/ZoomItModuleInterface/dllmain.cpp

@@ -13,8 +13,6 @@
 #include <shellapi.h>
 #include <common/interop/shared_constants.h>
 
-#include <atomic>
-
 namespace NonLocalizable
 {
     const wchar_t ModulePath[] = L"PowerToys.ZoomIt.exe";
@@ -109,7 +107,7 @@ public:
     // Returns if the powertoy is enabled
     virtual bool is_enabled() override
     {
-        return m_enabled.load();
+        return m_enabled;
     }
 
     // Destroy the powertoy and free memory
@@ -134,70 +132,10 @@ private:
         return false;
     }
 
-    // Called by the thread pool when the ZoomIt process exits.
-    static void CALLBACK OnZoomItProcessExited(PVOID lpParameter, BOOLEAN /*TimerOrWaitFired*/)
-    {
-        if (lpParameter == nullptr)
-        {
-            return;
-        }
-        reinterpret_cast<ZoomItModuleInterface*>(lpParameter)->HandleUnexpectedExit();
-    }
-
-    // Handles an unexpected ZoomIt process exit by restarting it when still enabled.
-    void HandleUnexpectedExit()
-    {
-        // Atomically clear the restart flag. If it was already cleared (e.g. by Disable()),
-        // another thread got here first or an intentional exit is in progress — do nothing.
-        if (!m_restartOnCrash.exchange(false))
-        {
-            return;
-        }
-
-        // The wait handle is auto-deregistered because WT_EXECUTEONLYONCE was used.
-        m_processWaitHandle = NULL;
-
-        if (!m_enabled.load())
-        {
-            // Disable() was called concurrently; do not restart.
-            return;
-        }
-
-        Logger::error(L"ZoomIt process exited unexpectedly. Attempting to restart...");
-
-        // Atomically take ownership of the stale process handle to avoid a double-close
-        // race with Disable(), then release it.
-        HANDLE hOldProcess = reinterpret_cast<HANDLE>(
-            InterlockedExchangePointer(reinterpret_cast<PVOID*>(&m_hProcess), nullptr));
-        if (hOldProcess)
-        {
-            CloseHandle(hOldProcess);
-        }
-
-        Enable();
-    }
-
     void Enable()
     {
         m_enabled = true;
 
-        // Prevent any in-flight callback from the previous session from triggering a
-        // spurious restart, then cancel the old wait (non-blocking, as Enable() may
-        // itself be called from the callback thread).
-        m_restartOnCrash = false;
-        if (m_processWaitHandle)
-        {
-            UnregisterWait(m_processWaitHandle);
-            m_processWaitHandle = NULL;
-        }
-
-        // Close any leftover process handle from a previous run.
-        if (m_hProcess)
-        {
-            CloseHandle(m_hProcess);
-            m_hProcess = nullptr;
-        }
-
         // Log telemetry
         Trace::EnableZoomIt(true);
 
@@ -226,47 +164,14 @@ private:
         else
         {
             m_hProcess = sei.hProcess;
-
-            // Register a one-shot thread-pool wait so we are notified if ZoomIt exits
-            // unexpectedly (e.g. due to a crash) and can restart it automatically.
-            m_restartOnCrash = true;
-            if (!RegisterWaitForSingleObject(
-                    &m_processWaitHandle,
-                    m_hProcess,
-                    OnZoomItProcessExited,
-                    this,
-                    INFINITE,
-                    WT_EXECUTEONLYONCE))
-            {
-                m_restartOnCrash = false;
-                Logger::error(L"Failed to register ZoomIt crash monitor: {}", get_last_error_or_default(GetLastError()));
-            }
         }
+
     }
 
     void Disable(bool const traceEvent)
     {
         m_enabled = false;
 
-        // Signal that any pending crash-monitoring callback must not restart ZoomIt.
-        m_restartOnCrash = false;
-
-        // Unregister the crash-monitoring wait and block until any in-flight callback
-        // has completed, so we can safely proceed with process teardown without racing.
-        // Use InterlockedExchangePointer to atomically take ownership of the handle,
-        // preventing a TOCTOU race with a concurrently completing callback.
-        {
-            HANDLE waitHandle = reinterpret_cast<HANDLE>(
-                InterlockedExchangePointer(reinterpret_cast<PVOID*>(&m_processWaitHandle), nullptr));
-            if (waitHandle)
-            {
-                // INVALID_HANDLE_VALUE: block until all callbacks for this wait have returned.
-                // Must not be called from within a callback — Disable() always runs on the
-                // main/runner thread, so this is safe.
-                UnregisterWaitEx(waitHandle, INVALID_HANDLE_VALUE);
-            }
-        }
-
         // Log telemetry
         if (traceEvent)
         {
@@ -278,27 +183,21 @@ private:
 
         ResetEvent(m_reload_settings_event_handle);
 
+        // Wait for 1.5 seconds for the process to end correctly and stop etw tracer
+        WaitForSingleObject(m_hProcess, 1500);
+
+        // If process is still running, terminate it
         if (m_hProcess)
         {
-            // Atomically take ownership of the process handle to avoid a double-close
-            // race with HandleUnexpectedExit().
-            HANDLE hProcess = reinterpret_cast<HANDLE>(
-                InterlockedExchangePointer(reinterpret_cast<PVOID*>(&m_hProcess), nullptr));
-            if (hProcess)
-            {
-                // Wait for 1.5 seconds for the process to end correctly and stop etw tracer
-                WaitForSingleObject(hProcess, 1500);
-
-                // If process is still running, terminate it
-                TerminateProcess(hProcess, 0);
-                CloseHandle(hProcess);
-            }
+            TerminateProcess(m_hProcess, 0);
+            m_hProcess = nullptr;
         }
+
     }
 
     bool is_process_running()
     {
-        return m_hProcess != nullptr && WaitForSingleObject(m_hProcess, 0) == WAIT_TIMEOUT;
+        return WaitForSingleObject(m_hProcess, 0) == WAIT_TIMEOUT;
     }
 
     virtual void call_custom_action(const wchar_t* action) override
@@ -322,10 +221,8 @@ private:
     std::wstring app_name;
     std::wstring app_key; //contains the non localized key of the powertoy
 
-    std::atomic<bool> m_enabled{ false };
+    bool m_enabled = false;
     HANDLE m_hProcess = nullptr;
-    HANDLE m_processWaitHandle = NULL;
-    std::atomic<bool> m_restartOnCrash{ false };
 
     HANDLE m_reload_settings_event_handle = NULL;
     HANDLE m_exit_event_handle = NULL;

src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/ContentPageViewModel.cs

@@ -129,8 +129,8 @@ public partial class ContentPageViewModel : PageViewModel, ICommandBarContext
 
         UpdateDetails();
 
-        FetchContent();
         model.ItemsChanged += Model_ItemsChanged;
+        FetchContent();
 
         DoOnUiThread(
         () =>

src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/ContentTreeViewModel.cs

@@ -44,9 +44,9 @@ public partial class ContentTreeViewModel(ITreeContent _tree, WeakReference<IPag
             UpdateProperty(nameof(Root));
         }
 
-        FetchContent();
         model.PropChanged += Model_PropChanged;
         model.ItemsChanged += Model_ItemsChanged;
+        FetchContent();
     }
 
     // Theoretically, we should unify this with the one in CommandPalettePageViewModelFactory

src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Dock/DockBandViewModel.cs

@@ -209,8 +209,8 @@ public sealed partial class DockBandViewModel : ExtensionObjectViewModel
         var list = command.Model.Unsafe as IListPage;
         if (list is not null)
         {
-            InitializeFromList(list);
             list.ItemsChanged += HandleItemsChanged;
+            InitializeFromList(list);
         }
         else
         {

src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/ListViewModel.cs

@@ -957,8 +957,8 @@ public partial class ListViewModel : PageViewModel, IDisposable
         Filters?.InitializeProperties();
         UpdateProperty(nameof(Filters));
 
-        FetchItems(true);
         model.ItemsChanged += Model_ItemsChanged;
+        FetchItems(true);
     }
 
     private static IGridPropertiesViewModel? LoadGridPropertiesViewModel(IGridProperties? gridProperties)