Window: fix underlying platform not being destroyed

It appears at least with the XCB platform that in the case the Window
is destroyed immediately before the GuiApplication gets destroyed
we could end in a case where:
- GuiApplication processes the events one last time before destruction
- Trying to send event to now destroyed Window and PlatformWindow would
  crash

This revealed a few issues (XCB platform):
- LinuxXcbPlatformIntegration::window getter was expected to return null
  if passed an invalid key. Yet, it was using std::map::at which would
  throw if key out of range.
- The Window when being destroyed would not destroy not unregister the
  platform window against LinuxXcbPlatformIntegration thus leaving a
  dangling pointer in the map of platformWindows.
- The LinuxXcbPlatformEventLoop::processXcbEvents would not always check
  we still have a valid platformWindow when distributing events.

A test was added to trigger such a case. Note that it would only trigger
the issue randomly unless running with Valgrind where it gets triggered
every time.

In Win32PlatformWindow destroy() isn't called in the destructor anymore
to make it in-line with other platforms. Otherwise, destroy() for win32
window would be called twice in ~Window().

Change-Id: I851c42c97a3c6f5e6c8bc4142bde7f9f1e61b48a
Reviewed-on: https://codereview.kdab.com/c/kdab/kdutils/+/136870
Reviewed-by: Mike Krus <mike.krus@kdab.com>
Tested-by: Continuous Integration <build@kdab.com>
Reviewed-by: Sean Harmer <sean.harmer@kdab.com>

src/KDGui/platform/linux/xcb/linux_xcb_platform_event_loop.cpp

@@ -114,8 +114,10 @@ void LinuxXcbPlatformEventLoop::processXcbEvents()
             const auto w = configureEvent->width;
             const auto h = configureEvent->height;
             auto window = m_platformIntegration->window(configureEvent->window);
-            SPDLOG_LOGGER_DEBUG(m_logger, "{}: Resize of window to {} x {}", xcbEvent->sequence, w, h);
-            window->handleResize(w, h);
+            if (window) {
+                SPDLOG_LOGGER_DEBUG(m_logger, "{}: Resize of window to {} x {}", xcbEvent->sequence, w, h);
+                window->handleResize(w, h);
+            }
             break;
         }
 
@@ -136,6 +138,8 @@ void LinuxXcbPlatformEventLoop::processXcbEvents()
         case XCB_BUTTON_PRESS: {
             const auto buttonEvent = reinterpret_cast<xcb_button_press_event_t *>(xcbEvent);
             auto window = m_platformIntegration->window(buttonEvent->event);
+            if (window == nullptr)
+                break;
             auto button = buttonEvent->detail;
 
             switch (button) {
@@ -217,6 +221,8 @@ void LinuxXcbPlatformEventLoop::processXcbEvents()
         case XCB_BUTTON_RELEASE: {
             const auto buttonEvent = reinterpret_cast<xcb_button_release_event_t *>(xcbEvent);
             auto window = m_platformIntegration->window(buttonEvent->event);
+            if (window == nullptr)
+                break;
             SPDLOG_LOGGER_DEBUG(m_logger,
                                 "Mouse release event for button {} at pos ({}, {})",
                                 buttonEvent->detail,
@@ -233,6 +239,8 @@ void LinuxXcbPlatformEventLoop::processXcbEvents()
         case XCB_MOTION_NOTIFY: {
             const auto mouseMoveEvent = reinterpret_cast<xcb_motion_notify_event_t *>(xcbEvent);
             auto window = m_platformIntegration->window(mouseMoveEvent->event);
+            if (window == nullptr)
+                break;
             SPDLOG_LOGGER_DEBUG(m_logger,
                                 "{}: Mouse move event for button {} at pos ({}, {})",
                                 xcbEvent->sequence,

src/KDGui/platform/linux/xcb/linux_xcb_platform_integration.cpp

@@ -66,6 +66,14 @@ LinuxXkbKeyboard *LinuxXcbPlatformIntegration::keyboard()
     return m_keyboard.get();
 }
 
+LinuxXcbPlatformWindow *LinuxXcbPlatformIntegration::window(xcb_window_t xcbWindow) const
+{
+    auto it = m_windows.find(xcbWindow);
+    if (it == m_windows.end())
+        return nullptr;
+    return it->second;
+}
+
 LinuxXcbPlatformEventLoop *LinuxXcbPlatformIntegration::createPlatformEventLoopImpl()
 {
     // We ensure that the logger has been created here rather than in the ctor so that

src/KDGui/platform/linux/xcb/linux_xcb_platform_integration.h

@@ -46,7 +46,7 @@ class KDGUI_API LinuxXcbPlatformIntegration : public AbstractGuiPlatformIntegrat
         m_windows.insert({ xcbWindow, window });
     }
     void unregisterWindowForEvents(xcb_window_t xcbWindow) { m_windows.erase(xcbWindow); }
-    LinuxXcbPlatformWindow *window(xcb_window_t xcbWindow) { return m_windows.at(xcbWindow); }
+    LinuxXcbPlatformWindow *window(xcb_window_t xcbWindow) const;
 
 private:
     LinuxXcbPlatformEventLoop *createPlatformEventLoopImpl() override;

src/KDGui/platform/win32/win32_platform_window.cpp

@@ -169,7 +169,6 @@ Win32PlatformWindow::Win32PlatformWindow(Win32GuiPlatformIntegration *platformIn
 
 Win32PlatformWindow::~Win32PlatformWindow()
 {
-    destroy();
     delete m_rawInputData;
     m_rawInputData = nullptr;
     m_rawInputDataSize = 0;

src/KDGui/window.cpp

@@ -33,7 +33,11 @@ Window::Window()
     rawMouseInputEnabled.valueChanged().connect(&Window::onRawMouseInputEnabledChanged, this);
 }
 
-Window::~Window() = default;
+Window::~Window()
+{
+    if (m_platformWindow)
+        destroy();
+}
 
 Window::Window(Window &&other) noexcept = default;
 Window &Window::operator=(Window &&other) noexcept = default;
@@ -80,6 +84,7 @@ void Window::destroy()
         return;
 
     m_platformWindow->destroy();
+    m_platformWindow = nullptr;
 }
 
 void Window::registerEventReceiver(Object *receiver)

tests/auto/gui/window/tst_window.cpp

@@ -80,4 +80,23 @@ TEST_CASE("Creation")
         REQUIRE(w->platformWindow()->type() == AbstractPlatformWindow::Type::Cocoa);
 #endif
     }
+
+    SUBCASE("doesn't crash if Window destroyed with pending platform events")
+    {
+        // GIVEN
+        GuiApplication app;
+        auto window = std::make_unique<Window>();
+        window->width = 512;
+        window->height = 512;
+        window->create();
+
+        REQUIRE(window->platformWindow() != nullptr);
+
+        // WHEN -> Trigger a resize (this should trigger platform events, at least it does for XCB)
+        window->width = 256;
+        window->height = 256;
+
+        // THEN -> Shouldn't crash if platform backend is trying to deliver resize events on app exit
+        // to window that is now destroyed
+    }
 }