From 58b0a14744b81bf92f8af63880224f1404641866 Mon Sep 17 00:00:00 2001 From: navjack Date: Mon, 14 Sep 2026 12:30:25 -0400 Subject: [PATCH] Apple: keep CocoaWindow main-queue updates valid after destruction Blocks queued to the main queue captured `this`, so a swapchain destroyed before an asynchronous size or refresh-rate update ran left the block reading freed memory (objc_msgSend crash in updateWindowAttributesInternal). Keep the cached state in a shared_ptr and capture it with the NSWindow instead. Co-Authored-By: Claude Opus 5 --- plume_apple.h | 23 +++++++---- plume_apple.mm | 101 +++++++++++++++++++++++-------------------------- 2 files changed, 63 insertions(+), 61 deletions(-) diff --git a/plume_apple.h b/plume_apple.h index 731d3c4..ae08261 100644 --- a/plume_apple.h +++ b/plume_apple.h @@ -8,6 +8,7 @@ #pragma once #include +#include #include #include "plume_render_interface_types.h" @@ -20,13 +21,21 @@ namespace plume { }; class CocoaWindow { - void* windowHandle; - CocoaWindowAttributes cachedAttributes; - std::atomic cachedRefreshRate; - mutable std::mutex attributesMutex; - - void updateWindowAttributesInternal(bool forceSync = false); - void updateRefreshRateInternal(bool forceSync = false); + public: + // Updates queued to the main thread capture this state instead of `this`, + // so they stay valid if the wrapper is destroyed before they run. + struct SharedState { + void* windowHandle = nullptr; + CocoaWindowAttributes cachedAttributes = {0, 0, 0, 0}; + std::atomic cachedRefreshRate{0}; + std::mutex attributesMutex; + }; + + private: + std::shared_ptr state; + + void updateWindowAttributesInternal(bool forceSync = false) const; + void updateRefreshRateInternal(bool forceSync = false) const; public: CocoaWindow(void* window); ~CocoaWindow(); diff --git a/plume_apple.mm b/plume_apple.mm index 1384fe9..9b45371 100644 --- a/plume_apple.mm +++ b/plume_apple.mm @@ -42,7 +42,7 @@ RenderDeviceVendor getRenderDeviceVendor(uint64_t registryID) { } IOObjectRelease(entry); // Release the entry if we couldn't get parent } - + return RenderDeviceVendor::UNKNOWN; } @@ -56,24 +56,35 @@ CGFloat getScaleFactor(NSWindow *nsWindow) { // MARK: - CocoaWindow - CocoaWindow::CocoaWindow(void* window) - : windowHandle(window), cachedRefreshRate(0) { - cachedAttributes = {0, 0, 0, 0}; + // Main thread only. Asynchronous callers pass shared state and an NSWindow + // captured by the block, never `this`: a swapchain can destroy its wrapper + // before a queued update runs. + static void updateAttributes(CocoaWindow::SharedState &state, NSWindow *nsWindow) { + NSRect contentFrame = [[nsWindow contentView] frame]; + CGFloat scaleFactor = getScaleFactor(nsWindow); + + std::lock_guard lock(state.attributesMutex); + state.cachedAttributes.x = (int)round(contentFrame.origin.x); + state.cachedAttributes.y = (int)round(contentFrame.origin.y); + state.cachedAttributes.width = (int)round(contentFrame.size.width * scaleFactor); + state.cachedAttributes.height = (int)round(contentFrame.size.height * scaleFactor); + } - if ([NSThread isMainThread]) { - NSWindow *nsWindow = (__bridge NSWindow *)windowHandle; - NSRect contentFrame = [[nsWindow contentView] frame]; - CGFloat scaleFactor = getScaleFactor(nsWindow); + static void updateRefreshRate(CocoaWindow::SharedState &state, NSWindow *nsWindow) { + NSScreen *screen = [nsWindow screen]; + if (@available(macOS 12.0, *)) { + state.cachedRefreshRate.store((int)[screen maximumFramesPerSecond]); + } + } - cachedAttributes.x = (int)round(contentFrame.origin.x); - cachedAttributes.y = (int)round(contentFrame.origin.y); - cachedAttributes.width = (int)round(contentFrame.size.width * scaleFactor); - cachedAttributes.height = (int)round(contentFrame.size.height * scaleFactor); + CocoaWindow::CocoaWindow(void* window) + : state(std::make_shared()) { + state->windowHandle = window; - NSScreen *screen = [nsWindow screen]; - if (@available(macOS 12.0, *)) { - cachedRefreshRate.store((int)[screen maximumFramesPerSecond]); - } + if ([NSThread isMainThread]) { + NSWindow *nsWindow = (__bridge NSWindow *)window; + updateAttributes(*state, nsWindow); + updateRefreshRate(*state, nsWindow); } else { updateWindowAttributesInternal(true); updateRefreshRateInternal(true); @@ -82,17 +93,11 @@ CGFloat getScaleFactor(NSWindow *nsWindow) { CocoaWindow::~CocoaWindow() {} - void CocoaWindow::updateWindowAttributesInternal(bool forceSync) { + void CocoaWindow::updateWindowAttributesInternal(bool forceSync) const { + std::shared_ptr sharedState = state; + NSWindow *nsWindow = (__bridge NSWindow *)sharedState->windowHandle; auto updateBlock = ^{ - NSWindow *nsWindow = (__bridge NSWindow *)windowHandle; - NSRect contentFrame = [[nsWindow contentView] frame]; - CGFloat scaleFactor = getScaleFactor(nsWindow); - - std::lock_guard lock(attributesMutex); - cachedAttributes.x = (int)round(contentFrame.origin.x); - cachedAttributes.y = (int)round(contentFrame.origin.y); - cachedAttributes.width = (int)round(contentFrame.size.width * scaleFactor); - cachedAttributes.height = (int)round(contentFrame.size.height * scaleFactor); + updateAttributes(*sharedState, nsWindow); }; if (forceSync) { @@ -102,13 +107,11 @@ CGFloat getScaleFactor(NSWindow *nsWindow) { } } - void CocoaWindow::updateRefreshRateInternal(bool forceSync) { + void CocoaWindow::updateRefreshRateInternal(bool forceSync) const { + std::shared_ptr sharedState = state; + NSWindow *nsWindow = (__bridge NSWindow *)sharedState->windowHandle; auto updateBlock = ^{ - NSWindow *nsWindow = (__bridge NSWindow *)windowHandle; - NSScreen *screen = [nsWindow screen]; - if (@available(macOS 12.0, *)) { - cachedRefreshRate.store((int)[screen maximumFramesPerSecond]); - } + updateRefreshRate(*sharedState, nsWindow); }; if (forceSync) { @@ -120,57 +123,47 @@ CGFloat getScaleFactor(NSWindow *nsWindow) { void CocoaWindow::getWindowAttributes(CocoaWindowAttributes* attributes) const { if ([NSThread isMainThread]) { - NSWindow *nsWindow = (__bridge NSWindow *)windowHandle; - NSRect contentFrame = [[nsWindow contentView] frame]; - CGFloat scaleFactor = getScaleFactor(nsWindow); - - { - std::lock_guard lock(attributesMutex); - const_cast(this)->cachedAttributes.x = (int)round(contentFrame.origin.x); - const_cast(this)->cachedAttributes.y = (int)round(contentFrame.origin.y); - const_cast(this)->cachedAttributes.width = (int)round(contentFrame.size.width * scaleFactor); - const_cast(this)->cachedAttributes.height = (int)round(contentFrame.size.height * scaleFactor); + updateAttributes(*state, (__bridge NSWindow *)state->windowHandle); - *attributes = cachedAttributes; - } + std::lock_guard lock(state->attributesMutex); + *attributes = state->cachedAttributes; } else { { - std::lock_guard lock(attributesMutex); - *attributes = cachedAttributes; + std::lock_guard lock(state->attributesMutex); + *attributes = state->cachedAttributes; } - const_cast(this)->updateWindowAttributesInternal(false); + updateWindowAttributesInternal(false); } } int CocoaWindow::getRefreshRate() const { if ([NSThread isMainThread]) { - NSWindow *nsWindow = (__bridge NSWindow *)windowHandle; + NSWindow *nsWindow = (__bridge NSWindow *)state->windowHandle; NSScreen *screen = [nsWindow screen]; if (@available(macOS 12.0, *)) { int freshRate = (int)[screen maximumFramesPerSecond]; - const_cast(this)->cachedRefreshRate.store(freshRate); + state->cachedRefreshRate.store(freshRate); return freshRate; } - return cachedRefreshRate.load(); + return state->cachedRefreshRate.load(); } else { - int rate = cachedRefreshRate.load(); + int rate = state->cachedRefreshRate.load(); - const_cast(this)->updateRefreshRateInternal(false); + updateRefreshRateInternal(false); return rate; } } void CocoaWindow::toggleFullscreen() { + NSWindow *nsWindow = (__bridge NSWindow *)state->windowHandle; if ([NSThread isMainThread]) { - NSWindow *nsWindow = (__bridge NSWindow *)windowHandle; [nsWindow toggleFullScreen:NULL]; } else { dispatch_async(dispatch_get_main_queue(), ^{ - NSWindow *nsWindow = (__bridge NSWindow *)windowHandle; [nsWindow toggleFullScreen:NULL]; }); }