diff --git a/src/core/shellhandler.cpp b/src/core/shellhandler.cpp index 459cfba62..a2cdb5179 100644 --- a/src/core/shellhandler.cpp +++ b/src/core/shellhandler.cpp @@ -186,6 +186,12 @@ void ShellHandler::createPrelaunchSplash(const QString &appId, iconBuffer->unlock(); } m_prelaunchWrappers.append(wrapper); + // Guard against dangling pointers: no matter how the wrapper is destroyed + // (timeout, close request, match, workspace cleanup, parent destruction, etc.), + // remove it from the prelaunch list. + connect(wrapper, &QObject::destroyed, this, [this, wrapper]() { + m_prelaunchWrappers.removeAll(wrapper); + }); m_workspace->addSurface(wrapper); setupSurfaceActiveWatcher(wrapper); registerSurfaceToForeignToplevel(wrapper); diff --git a/src/modules/capture/capture.cpp b/src/modules/capture/capture.cpp index d9dd6bf6a..0e2bba17c 100644 --- a/src/modules/capture/capture.cpp +++ b/src/modules/capture/capture.cpp @@ -580,7 +580,10 @@ void CaptureSourceSelector::doneSelection() this, &CaptureSourceSelector::createImage); m_internalContentItem->setVisible(false); - m_canvas->surfaceItem()->setSubsurfacesVisible(false); + // m_canvas may be null when no mask surface exists (e.g. capture via + // xdg-desktop-portal without a mask). Guard against null dereference. + if (m_canvas && m_canvas->surfaceItem()) + m_canvas->surfaceItem()->setSubsurfacesVisible(false); } void CaptureSourceSelector::cancelSelection() diff --git a/src/modules/capture/impl/capturev1impl.cpp b/src/modules/capture/impl/capturev1impl.cpp index dcc63f8d5..5dadf36c0 100644 --- a/src/modules/capture/impl/capturev1impl.cpp +++ b/src/modules/capture/impl/capturev1impl.cpp @@ -138,19 +138,26 @@ treeland_capture_manager_v1::treeland_capture_manager_v1(wl_display *display, QO void treeland_capture_manager_v1::addClientResource(wl_client *client, wl_resource *resource) { WClient *wClient = WClient::get(client); + // Do NOT connect WClient::destroyed to wl_resource_destroy here. + // When wl_client_destroy runs, it iterates the client's resource map + // and destroys each resource. Calling wl_resource_destroy from within + // a WClient::destroyed handler (which fires during that iteration) + // corrupts the resource map and causes "double free or corruption". + // The resource will be automatically cleaned up by libwayland. + // We only need to track the entry for explicit destroy requests and + // remove stale entries when the client goes away. connect(wClient, &WClient::destroyed, this, [this, wClient, resource]() { - destroyClientResource(wClient, resource); + clientResources.removeOne({ wClient, resource }); }); clientResources.push_back({ wClient, resource }); } void treeland_capture_manager_v1::destroyClientResource(WClient *client, wl_resource *resource) { - for (const auto &pair : clientResources) { - if (pair.first == client && pair.second == resource) { - wl_resource_destroy(pair.second); - clientResources.removeOne(pair); - } + // Called from the explicit protocol destroy request only (not during client teardown). + QPair entry{ client, resource }; + if (clientResources.removeOne(entry)) { + wl_resource_destroy(resource); } } @@ -292,8 +299,12 @@ void treeland_capture_context_v1::sendSourceReady(QRect region, uint32_t source_ void treeland_capture_context_v1::setResource(wl_client *client, wl_resource *resource) { WClient *wClient = WClient::get(client); + // Do NOT call wl_resource_destroy from WClient::destroyed. + // libwayland already destroys all client resources during wl_client_destroy; + // doing so again from this handler corrupts the resource map ("double free"). + // Just null out the pointer so we don't send on a stale resource. connect(wClient, &WClient::destroyed, this, [this] { - wl_resource_destroy(this->resource); + this->resource = nullptr; }); this->resource = resource; } @@ -321,8 +332,11 @@ void handle_treeland_capture_frame_v1_copy(wl_client *client, void treeland_capture_session_v1::setResource(wl_client *client, wl_resource *resource) { WClient *wClient = WClient::get(client); + // Do NOT call wl_resource_destroy from WClient::destroyed. + // libwayland already destroys all client resources during wl_client_destroy; + // doing so again from this handler corrupts the resource map ("double free"). connect(wClient, &WClient::destroyed, this, [this] { - wl_resource_destroy(this->resource); + this->resource = nullptr; }); this->resource = resource; } @@ -348,8 +362,11 @@ void treeland_capture_session_v1::sendSourceResizeCancel() void treeland_capture_frame_v1::setResource(wl_client *client, wl_resource *resource) { WClient *wClient = WClient::get(client); + // Do NOT call wl_resource_destroy from WClient::destroyed. + // libwayland already destroys all client resources during wl_client_destroy; + // doing so again from this handler corrupts the resource map ("double free"). connect(wClient, &WClient::destroyed, this, [this] { - wl_resource_destroy(this->resource); + this->resource = nullptr; }); this->resource = resource; } diff --git a/src/modules/foreign-toplevel/impl/foreign_toplevel_manager_impl.cpp b/src/modules/foreign-toplevel/impl/foreign_toplevel_manager_impl.cpp index 495fd4cae..975063b5a 100644 --- a/src/modules/foreign-toplevel/impl/foreign_toplevel_manager_impl.cpp +++ b/src/modules/foreign-toplevel/impl/foreign_toplevel_manager_impl.cpp @@ -817,10 +817,22 @@ static void treeland_foreign_toplevel_manager_handle_get_dock_preview_context( context->destroy_listener_wrapper.context = context; context->destroy_listener_wrapper.wrapped_listener.notify = [](struct wl_listener *listener, [[maybe_unused]] void *data) { treeland_dock_preview_context_v1::TDPCPODWrapper *wrapper = wl_container_of(listener, wrapper, wrapped_listener); - wl_resource_destroy(wrapper->context->resource); - - // wl_list_remove(&context->destroy_listener.link); - // context->relative_surface = nullptr; + // Only remove the listener from the signal and clear the surface reference. + // Do NOT call wl_resource_destroy() here: if the relative_surface belongs + // to the same wl_client as the dock preview context, we would modify the + // client's resource map during wl_map_for_each iteration inside + // wl_client_destroy, corrupting the map and causing a crash (null pointer + // in wl_signal_emit). Even for cross-client cases, calling + // wl_resource_destroy from within a wl_signal_emit on the surface's + // destroy signal is unsafe because the subsequent + // treeland_dock_preview_context_resource_destroy would wl_list_remove the + // listener that is currently being dispatched. + // + // The context resource will be cleaned up when the dock client disconnects + // or explicitly calls the destroy request. + wl_list_remove(&wrapper->wrapped_listener.link); + wl_list_init(&wrapper->wrapped_listener.link); + wrapper->context->relative_surface = nullptr; }; wl_signal_add(&context->relative_surface->events.destroy, &context->destroy_listener_wrapper.wrapped_listener); diff --git a/src/modules/personalization/impl/appearance_impl.cpp b/src/modules/personalization/impl/appearance_impl.cpp index bca451ba0..f63747282 100644 --- a/src/modules/personalization/impl/appearance_impl.cpp +++ b/src/modules/personalization/impl/appearance_impl.cpp @@ -98,7 +98,10 @@ personalization_appearance_context_v1::personalization_appearance_context_v1( this, [](struct wl_resource *resource) { auto *p = personalization_appearance_context_v1::fromResource(resource); + if (!p) + return; Q_EMIT p->beforeDestroy(); + wl_resource_set_user_data(resource, nullptr); delete p; }); diff --git a/src/modules/personalization/impl/personalization_manager_impl.cpp b/src/modules/personalization/impl/personalization_manager_impl.cpp index b11670e34..8f15d3074 100644 --- a/src/modules/personalization/impl/personalization_manager_impl.cpp +++ b/src/modules/personalization/impl/personalization_manager_impl.cpp @@ -90,7 +90,7 @@ personalization_window_context_v1::~personalization_window_context_v1() static void personalization_window_context_resource_destroy(struct wl_resource *resource) { - if (resource) + if (!resource) return; auto window_context = personalization_window_context_v1::from_resource(resource); @@ -99,7 +99,9 @@ static void personalization_window_context_resource_destroy(struct wl_resource * } delete window_context; - wl_list_remove(wl_resource_get_link(resource)); + // Do NOT call wl_list_remove here — libwayland's remove_and_destroy_resource + // already removes the resource link from the list after calling this callback. + // Calling it here would double-remove and corrupt the list. } namespace Personalization { @@ -222,7 +224,7 @@ personalization_wallpaper_context_v1::~personalization_wallpaper_context_v1() static void personalization_wallpaper_context_resource_destroy(struct wl_resource *resource) { - if (resource) + if (!resource) return; auto wallpaper_context = personalization_wallpaper_context_v1::from_resource(resource); @@ -231,7 +233,7 @@ static void personalization_wallpaper_context_resource_destroy(struct wl_resourc } delete wallpaper_context; - wl_list_remove(wl_resource_get_link(resource)); + // Do NOT call wl_list_remove here — libwayland handles it. } void set_cursor_theme([[maybe_unused]] struct wl_client *client, struct wl_resource *resource, const char *name) @@ -312,7 +314,7 @@ personalization_cursor_context_v1::~personalization_cursor_context_v1() static void personalization_cursor_context_resource_destroy(struct wl_resource *resource) { - if (resource) + if (!resource) return; auto cursor_context = personalization_cursor_context_v1::from_resource(resource); @@ -321,7 +323,7 @@ static void personalization_cursor_context_resource_destroy(struct wl_resource * } delete cursor_context; - wl_list_remove(wl_resource_get_link(resource)); + // Do NOT call wl_list_remove here — libwayland handles it. } void create_personalization_window_context_listener(struct wl_client *client, @@ -360,16 +362,11 @@ treeland_personalization_manager_v1 *treeland_personalization_manager_v1::from_r static void treeland_personalization_manager_resource_destroy(struct wl_resource *resource) { - if (resource) - return; - - auto manager = treeland_personalization_manager_v1::from_resource(resource); - if (!manager) { - return; - } - - delete manager; - wl_list_remove(wl_resource_get_link(resource)); + // Do not delete the manager — it is a singleton owned by the display. + // Do not call wl_list_remove here — libwayland's remove_and_destroy_resource + // already calls wl_list_remove(&resource->link) after this callback returns. + // Doing it here would double-remove and corrupt the linked list. + Q_UNUSED(resource); } void create_personalization_window_context_listener(struct wl_client *client, diff --git a/src/modules/personalization/personalizationmanager.cpp b/src/modules/personalization/personalizationmanager.cpp index c91d00be8..1306da85d 100644 --- a/src/modules/personalization/personalizationmanager.cpp +++ b/src/modules/personalization/personalizationmanager.cpp @@ -196,26 +196,32 @@ void PersonalizationV1::onAppearanceContextCreated(personalization_appearance_co connect(context, &Appearance::roundCornerRadiusChanged, this, [this](int32_t radius) { Helper::instance()->config()->setWindowRadius(radius); - for (auto *context : m_appearanceContexts) { - context->sendRoundCornerRadius(radius); + // Copy the list to guard against re-entrant modification if a send + // triggers client destruction (which removes entries from the vector). + const auto contexts = m_appearanceContexts; + for (auto *ctx : contexts) { + ctx->sendRoundCornerRadius(radius); } }); connect(context, &Appearance::iconThemeChanged, this, [this](const QString &theme) { Helper::instance()->config()->setIconThemeName(theme); - for (auto *context : m_appearanceContexts) { - context->sendIconTheme(theme.toUtf8()); + const auto contexts = m_appearanceContexts; + for (auto *ctx : contexts) { + ctx->sendIconTheme(theme.toUtf8()); } }); connect(context, &Appearance::activeColorChanged, this, [this](const QString &color) { Helper::instance()->config()->setActiveColor(color); - for (auto *context : m_appearanceContexts) { - context->sendActiveColor(color.toUtf8()); + const auto contexts = m_appearanceContexts; + for (auto *ctx : contexts) { + ctx->sendActiveColor(color.toUtf8()); } }); connect(context, &Appearance::windowOpacityChanged, this, [this](uint32_t opacity) { Helper::instance()->config()->setWindowOpacity(opacity); - for (auto *context : m_appearanceContexts) { - context->sendWindowOpacity(opacity); + const auto contexts = m_appearanceContexts; + for (auto *ctx : contexts) { + ctx->sendWindowOpacity(opacity); } }); connect(context, &Appearance::windowThemeTypeChanged, this, [this](int32_t type) { @@ -223,14 +229,16 @@ void PersonalizationV1::onAppearanceContextCreated(personalization_appearance_co if (dconfigType.has_value()) { Helper::instance()->config()->setWindowThemeType(*dconfigType); } - for (auto *context : m_appearanceContexts) { - context->sendWindowThemeType(type); + const auto contexts = m_appearanceContexts; + for (auto *ctx : contexts) { + ctx->sendWindowThemeType(type); } }); connect(context, &Appearance::titlebarHeightChanged, this, [this](uint32_t height) { Helper::instance()->config()->setWindowTitlebarHeight(height); - for (auto *context : m_appearanceContexts) { - context->sendWindowTitlebarHeight(height); + const auto contexts = m_appearanceContexts; + for (auto *ctx : contexts) { + ctx->sendWindowTitlebarHeight(height); } }); diff --git a/src/surface/surfacewrapper.cpp b/src/surface/surfacewrapper.cpp index 35340c933..46fbcc920 100644 --- a/src/surface/surfacewrapper.cpp +++ b/src/surface/surfacewrapper.cpp @@ -996,8 +996,21 @@ void SurfaceWrapper::markWrapperToRemoved() } m_subSurfaces.clear(); m_shellSurface = nullptr; - if (m_surfaceItem) + if (m_surfaceItem) { m_surfaceItem->disconnect(this); + // Hide the surfaceItem immediately to prevent the scene graph renderer + // from accessing stale wlr_surface/wlr_buffer data during the next + // render frame. The underlying wlroots resources are already freed at + // this point (the wayland client disconnected or the surface was + // destroyed), but deleteLater() won't run until the next event-loop + // iteration. Without hiding, the QSG renderer will traverse the still- + // visible WSurfaceItem and read freed GPU buffer memory, causing heap + // corruption ("corrupted size vs. prev_size"). + m_surfaceItem->setVisible(false); + } + + // Also hide the wrapper itself so the scene graph skips the entire subtree. + QQuickItem::setVisible(false); if (!isWindowAnimationRunning()) { deleteLater();