FancyZones: prevent WinHookEventProc reentrancy bugs by serializing events to FancyZones::WndProc

src/modules/fancyzones/dll/dllmain.cpp

@@ -10,6 +10,7 @@
 #include <lib/trace.h>
 #include <lib/Settings.h>
 #include <lib/FancyZones.h>
+#include <lib/FancyZonesWinHookEventIDs.h>
 
 extern "C" IMAGE_DOS_HEADER __ImageBase;
 
@@ -74,6 +75,7 @@ public:
     {
         if (!m_app)
         {
+            initialize_winhook_event_ids();
             Trace::FancyZones::EnableFancyZones(true);
             m_app = MakeFancyZones(reinterpret_cast<HINSTANCE>(&__ImageBase), m_settings);
 
@@ -186,9 +188,6 @@ private:
 
     intptr_t HandleKeyboardHookEvent(LowlevelKeyboardEvent* data) noexcept;
     void HandleWinHookEvent(WinHookEvent* data) noexcept;
-    void MoveSizeStart(HWND window, POINT const& ptScreen) noexcept;
-    void MoveSizeEnd(HWND window, POINT const& ptScreen) noexcept;
-    void MoveSizeUpdate(POINT const& ptScreen) noexcept;
 
     winrt::com_ptr<IFancyZones> m_app;
     winrt::com_ptr<IFancyZonesSettings> m_settings;
@@ -242,14 +241,12 @@ intptr_t FancyZonesModule::HandleKeyboardHookEvent(LowlevelKeyboardEvent* data)
 
 void FancyZonesModule::HandleWinHookEvent(WinHookEvent* data) noexcept
 {
-    POINT ptScreen;
+    auto fzCallback = m_app.as<IFancyZonesCallback>();
-    GetPhysicalCursorPos(&ptScreen);
-
     switch (data->event)
     {
     case EVENT_SYSTEM_MOVESIZESTART:
     {
-        MoveSizeStart(data->hwnd, ptScreen);
+        fzCallback->HandleWinHookEvent(data);
         if (!m_objectLocationWinEventHook)
         {
             m_objectLocationWinEventHook = SetWinEventHook(EVENT_OBJECT_LOCATIONCHANGE,
@@ -269,16 +266,13 @@ void FancyZonesModule::HandleWinHookEvent(WinHookEvent* data) noexcept
         {
             m_objectLocationWinEventHook = nullptr;
         }
-        MoveSizeEnd(data->hwnd, ptScreen);
+        fzCallback->HandleWinHookEvent(data);
     }
     break;
 
     case EVENT_OBJECT_LOCATIONCHANGE:
     {
-        if (m_app.as<IFancyZonesCallback>()->InMoveSize())
+        fzCallback->HandleWinHookEvent(data);
-        {
-            MoveSizeUpdate(ptScreen);
-        }
     }
     break;
 
@@ -298,10 +292,7 @@ void FancyZonesModule::HandleWinHookEvent(WinHookEvent* data) noexcept
     case EVENT_OBJECT_SHOW:
     case EVENT_OBJECT_CREATE:
     {
-        if (data->idObject == OBJID_WINDOW)
+        fzCallback->HandleWinHookEvent(data);
-        {
-            m_app.as<IFancyZonesCallback>()->WindowCreated(data->hwnd);
-        }
     }
     break;
 
@@ -310,27 +301,6 @@ void FancyZonesModule::HandleWinHookEvent(WinHookEvent* data) noexcept
     }
 }
 
-void FancyZonesModule::MoveSizeStart(HWND window, POINT const& ptScreen) noexcept
-{
-    if (auto monitor = MonitorFromPoint(ptScreen, MONITOR_DEFAULTTONULL))
-    {
-        m_app.as<IFancyZonesCallback>()->MoveSizeStart(window, monitor, ptScreen);
-    }
-}
-
-void FancyZonesModule::MoveSizeEnd(HWND window, POINT const& ptScreen) noexcept
-{
-    m_app.as<IFancyZonesCallback>()->MoveSizeEnd(window, ptScreen);
-}
-
-void FancyZonesModule::MoveSizeUpdate(POINT const& ptScreen) noexcept
-{
-    if (auto monitor = MonitorFromPoint(ptScreen, MONITOR_DEFAULTTONULL))
-    {
-        m_app.as<IFancyZonesCallback>()->MoveSizeUpdate(monitor, ptScreen);
-    }
-}
-
 extern "C" __declspec(dllexport) PowertoyModuleIface* __cdecl powertoy_create()
 {
     return new FancyZonesModule();

src/modules/fancyzones/lib/FancyZones.cpp

@@ -1,6 +1,8 @@
 #include "pch.h"
-#include "common/dpi_aware.h"
+
-#include "common/on_thread_executor.h"
+#include <common/dpi_aware.h>
+#include <common/on_thread_executor.h>
+#include <common/window_helpers.h>
 
 #include "FancyZones.h"
 #include "lib/Settings.h"
@@ -8,13 +10,12 @@
 #include "lib/JsonHelpers.h"
 #include "lib/ZoneSet.h"
 #include "lib/WindowMoveHandler.h"
+#include "lib/FancyZonesWinHookEventIDs.h"
+#include "lib/util.h"
 #include "trace.h"
 #include "VirtualDesktopUtils.h"
 
-#include <functional>
+#include <interface/win_hook_event_data.h>
-#include <common/window_helpers.h>
-#include <lib/util.h>
-#include <unordered_set>
 
 enum class DisplayChangeType
 {
@@ -80,6 +81,38 @@ public:
         std::unique_lock writeLock(m_lock);
         m_windowMoveHandler.MoveSizeEnd(window, ptScreen, m_zoneWindowMap);
     }
+    IFACEMETHODIMP_(void)
+    HandleWinHookEvent(const WinHookEvent* data) noexcept
+    {
+        const auto wparam = reinterpret_cast<WPARAM>(data->hwnd);
+        const LONG lparam = 0;
+        std::shared_lock readLock(m_lock);
+        switch (data->event)
+        {
+        case EVENT_SYSTEM_MOVESIZESTART:
+            PostMessageW(m_window, WM_PRIV_MOVESIZESTART, wparam, lparam);
+            break;
+        case EVENT_SYSTEM_MOVESIZEEND:
+            PostMessageW(m_window, WM_PRIV_MOVESIZEEND, wparam, lparam);
+            break;
+        case EVENT_OBJECT_LOCATIONCHANGE:
+            PostMessageW(m_window, WM_PRIV_LOCATIONCHANGE, wparam, lparam);
+            break;
+        case EVENT_OBJECT_NAMECHANGE:
+            PostMessageW(m_window, WM_PRIV_NAMECHANGE, wparam, lparam);
+            break;
+
+        case EVENT_OBJECT_UNCLOAKED:
+        case EVENT_OBJECT_SHOW:
+        case EVENT_OBJECT_CREATE:
+            if (data->idObject == OBJID_WINDOW)
+            {
+                PostMessageW(m_window, WM_PRIV_WINDOWCREATED, wparam, lparam);
+            }
+            break;
+        }
+    }
+
     IFACEMETHODIMP_(void)
     VirtualDesktopChanged() noexcept;
     IFACEMETHODIMP_(void)
@@ -263,8 +296,7 @@ FancyZones::Run() noexcept
         .wait();
 
     m_terminateVirtualDesktopTrackerEvent.reset(CreateEvent(nullptr, FALSE, FALSE, nullptr));
-    m_virtualDesktopTrackerThread.submit(OnThreadExecutor::task_t{ [&] {
+    m_virtualDesktopTrackerThread.submit(OnThreadExecutor::task_t{ [&] { VirtualDesktopUtils::HandleVirtualDesktopUpdates(m_window, WM_PRIV_VD_UPDATE, m_terminateVirtualDesktopTrackerEvent.get()); } });
-        VirtualDesktopUtils::HandleVirtualDesktopUpdates(m_window, WM_PRIV_VD_UPDATE, m_terminateVirtualDesktopTrackerEvent.get()); } });
 }
 
 // IFancyZones
@@ -289,8 +321,8 @@ FancyZones::Destroy() noexcept
 IFACEMETHODIMP_(void)
 FancyZones::VirtualDesktopChanged() noexcept
 {
-    // VirtualDesktopChanged is called from another thread but results in new windows being created.
+    // VirtualDesktopChanged is called from a reentrant WinHookProc function, therefore we must postpone the actual logic
-    // Jump over to the UI thread to handle it.
+    // until we're in FancyZones::WndProc, which is not reentrant.
     std::shared_lock readLock(m_lock);
     PostMessage(m_window, WM_PRIV_VD_SWITCH, 0, 0);
 }
@@ -557,6 +589,9 @@ LRESULT FancyZones::WndProc(HWND window, UINT message, WPARAM wparam, LPARAM lpa
 
     default:
     {
+        POINT ptScreen;
+        GetPhysicalCursorPos(&ptScreen);
+
         if (message == WM_PRIV_VD_INIT)
         {
             OnDisplayChange(DisplayChangeType::Initialization);
@@ -587,6 +622,31 @@ LRESULT FancyZones::WndProc(HWND window, UINT message, WPARAM wparam, LPARAM lpa
                 m_terminateEditorEvent.release();
             }
         }
+        else if (message == WM_PRIV_MOVESIZESTART)
+        {
+            auto hwnd = reinterpret_cast<HWND>(wparam);
+            if (auto monitor = MonitorFromPoint(ptScreen, MONITOR_DEFAULTTONULL))
+            {
+                MoveSizeStart(hwnd, monitor, ptScreen);
+            }
+        }
+        else if (message == WM_PRIV_MOVESIZEEND)
+        {
+            auto hwnd = reinterpret_cast<HWND>(wparam);
+            MoveSizeEnd(hwnd, ptScreen);
+        }
+        else if (message == WM_PRIV_LOCATIONCHANGE && InMoveSize())
+        {
+            if (auto monitor = MonitorFromPoint(ptScreen, MONITOR_DEFAULTTONULL))
+            {
+                MoveSizeUpdate(monitor, ptScreen);
+            }
+        }
+        else if (message == WM_PRIV_WINDOWCREATED)
+        {
+            auto hwnd = reinterpret_cast<HWND>(wparam);
+            WindowCreated(hwnd);
+        }
         else
         {
             return DefWindowProc(window, message, wparam, lparam);
@@ -672,7 +732,6 @@ void FancyZones::AddZoneWindow(HMONITOR monitor, PCWSTR deviceId) noexcept
     }
 }
 
-
 LRESULT CALLBACK FancyZones::s_WndProc(HWND window, UINT message, WPARAM wparam, LPARAM lparam) noexcept
 {
     auto thisRef = reinterpret_cast<FancyZones*>(GetWindowLongPtr(window, GWLP_USERDATA));

src/modules/fancyzones/lib/FancyZones.h

@@ -4,6 +4,8 @@ interface IZoneWindow;
 interface IFancyZonesSettings;
 interface IZoneSet;
 
+struct WinHookEvent;
+
 interface __declspec(uuid("{50D3F0F5-736E-4186-BDF4-3D6BEE150C3A}")) IFancyZones : public IUnknown
 {
     /**
@@ -54,6 +56,12 @@ interface __declspec(uuid("{2CB37E8F-87E6-4AEC-B4B2-E0FDC873343F}")) IFancyZones
      * Inform FancyZones that user has switched between virtual desktops.
      */
     IFACEMETHOD_(void, VirtualDesktopChanged)() = 0;
+    /**
+     * Callback from WinEventHook to FancyZones
+     *
+     * @param   data  Handle of window being moved or resized.
+     */
+    IFACEMETHOD_(void, HandleWinHookEvent)(const WinHookEvent* data){};
     /**
      * Inform FancyZones that new window is created. FancyZones will try to assign it to the
      * zone insde active zone layout (if information about last zone, in which window was located

src/modules/fancyzones/lib/FancyZonesLib.vcxproj

@@ -95,6 +95,7 @@
   </ItemDefinitionGroup>
   <ItemGroup>
     <ClInclude Include="FancyZones.h" />
+    <ClInclude Include="FancyZonesWinHookEventIDs.h" />
     <ClInclude Include="JsonHelpers.h" />
     <ClInclude Include="pch.h" />
     <ClInclude Include="resource.h" />
@@ -109,6 +110,7 @@
   </ItemGroup>
   <ItemGroup>
     <ClCompile Include="FancyZones.cpp" />
+    <ClCompile Include="FancyZonesWinHookEventIDs.cpp" />
     <ClCompile Include="JsonHelpers.cpp" />
     <ClCompile Include="pch.cpp">
       <PrecompiledHeader Condition="'$(Configuration)|$(Platform)'=='Debug|x64'">Create</PrecompiledHeader>

src/modules/fancyzones/lib/FancyZonesLib.vcxproj.filters

@@ -51,6 +51,9 @@
     <ClInclude Include="WindowMoveHandler.h">
       <Filter>Header Files</Filter>
     </ClInclude>
+    <ClInclude Include="FancyZonesWinHookEventIDs.h">
+      <Filter>Header Files</Filter>
+    </ClInclude>
   </ItemGroup>
   <ItemGroup>
     <ClCompile Include="pch.cpp">
@@ -86,6 +89,9 @@
     <ClCompile Include="WindowMoveHandler.cpp">
       <Filter>Source Files</Filter>
     </ClCompile>
+    <ClCompile Include="FancyZonesWinHookEventIDs.cpp">
+      <Filter>Source Files</Filter>
+    </ClCompile>
   </ItemGroup>
   <ItemGroup>
     <ResourceCompile Include="fancyzones.rc">

src/modules/fancyzones/lib/FancyZonesWinHookEventIDs.cpp

@@ -0,0 +1,24 @@
+#include "pch.h"
+
+#include <mutex>
+
+#include "FancyZonesWinHookEventIDs.h"
+
+UINT WM_PRIV_MOVESIZESTART;
+UINT WM_PRIV_MOVESIZEEND;
+UINT WM_PRIV_LOCATIONCHANGE;
+UINT WM_PRIV_NAMECHANGE;
+UINT WM_PRIV_WINDOWCREATED;
+
+std::once_flag init_flag;
+
+void initialize_winhook_event_ids()
+{
+    std::call_once(init_flag, [&] {
+        WM_PRIV_MOVESIZESTART = RegisterWindowMessage(L"{f48def23-df42-4c0f-a13d-3eb4a9e204d4}");
+        WM_PRIV_MOVESIZEEND = RegisterWindowMessage(L"{805d643c-804d-4728-b533-907d760ebaf0}");
+        WM_PRIV_LOCATIONCHANGE = RegisterWindowMessage(L"{d56c5ee7-58e5-481c-8c4f-8844cf4d0347}");
+        WM_PRIV_NAMECHANGE = RegisterWindowMessage(L"{b7b30c61-bfa0-4d95-bcde-fc4f2cbf6d76}");
+        WM_PRIV_WINDOWCREATED = RegisterWindowMessage(L"{bdb10669-75da-480a-9ec4-eeebf09a02d7}");
+    });
+}

src/modules/fancyzones/lib/FancyZonesWinHookEventIDs.h

@@ -0,0 +1,9 @@
+#pragma once
+
+extern UINT WM_PRIV_MOVESIZESTART;
+extern UINT WM_PRIV_MOVESIZEEND;
+extern UINT WM_PRIV_LOCATIONCHANGE;
+extern UINT WM_PRIV_NAMECHANGE;
+extern UINT WM_PRIV_WINDOWCREATED;
+
+void initialize_winhook_event_ids();

src/modules/fancyzones/lib/pch.h

@@ -17,6 +17,8 @@
 #include <winrt/windows.foundation.h>
 #include <psapi.h>
 #include <shared_mutex>
+#include <functional>
+#include <unordered_set>
 
 #pragma comment(lib, "windowsapp")