-
Notifications
You must be signed in to change notification settings - Fork 33
fix: normal termination for Treeland #731
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
deepin-ci-robot
commented
Feb 3, 2026
[APPROVALNOTIFIER] This PR is NOT APPROVED
This pull-request has been approved by: misaka18931
The full list of commands accepted by this bot can be found here.
Details
Needs approval from an approver in each of these files:Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment
Reviewer's GuideAdjusts scene graph and renderer cleanup, XWayland teardown, and Treeland shutdown sequencing to ensure normal termination and safer resource destruction, plus adds a temporary emergency restart keybinding. Sequence diagram for scene graph invalidation cleanupsequenceDiagram
participant QQuickWindow
participant WBufferRenderer
participant RhiManager
participant QSGRenderer
%% WBufferRenderer connection
QQuickWindow->>WBufferRenderer: sceneGraphInvalidated
activate WBufferRenderer
WBufferRenderer->>WBufferRenderer: invalidateSceneGraph()
WBufferRenderer->>WBufferRenderer: m_textureProvider.reset()
loop for each Source in m_sourceList
WBufferRenderer->>QSGRenderer: delete Source.renderer
WBufferRenderer->>WBufferRenderer: Source.renderer = nullptr
end
deactivate WBufferRenderer
%% RhiManager connection
QQuickWindow->>RhiManager: sceneGraphInvalidated
activate RhiManager
RhiManager->>RhiManager: delete this
deactivate RhiManager
destroy RhiManager
RhiManager-->>QSGRenderer: ~RhiManager()
destroy QSGRenderer
Sequence diagram for WXWayland destruction and XWayland teardownsequenceDiagram
participant Session
participant Helper
participant ShellHandler
participant WXWayland
participant QWXWayland
participant wlr_xwayland
Session->>Session: ~Session()
alt Helper instance exists
Session->>Helper: Helper::instance()
Helper-->>Session: pointer
Session->>Helper: shellHandler()
Helper-->>Session: ShellHandler*
Session->>ShellHandler: removeXWayland(m_xwayland)
ShellHandler->>WXWayland: destroy(WServer*)
WXWayland->>QWXWayland: handle()
WXWayland->>QWXWayland: destroy()
QWXWayland->>wlr_xwayland: wlr_xwayland_destroy()
WXWayland->>WXWayland: clear surfaceList
WXWayland-->>ShellHandler: destruction complete
ShellHandler-->>Session: WXWayland removed
Session->>Session: m_xwayland = nullptr
else Helper instance is null
Session->>Session: m_xwayland kept for WServer cleanup
Session->>Session: m_xwayland = nullptr
end
Updated class diagram for renderer, XWayland, and shutdown managementclassDiagram
class WBufferRenderer {
+WBufferRenderer(QQuickItem* parent)
+~WBufferRenderer()
+invalidateSceneGraph() void
+releaseResources() void
-cleanTextureProvider() void
-resetSources() void
-renderWindow() WOutputRenderWindow*
-outputWindow() WOutputRenderWindow*
-m_textureProvider
-m_sourceList
}
class RhiManager {
+RhiManager(QQuickWindow* owner)
+~RhiManager()
-renderer QSGRenderer*
-isBatchRenderer bool
}
class Helper {
+Helper(QObject* parent)
+~Helper()
+beforeDisposeEvent(WSeat* seat, QWindow* window, QInputEvent* event) bool
+addSocket(WSocket* socket) void
+createXWayland() WXWayland*
+personalization() PersonalizationV1*
+shellHandler() ShellHandler*
+static instance() Helper*
-m_instance static Helper*
+requestQuit()
}
class Session {
+Session(QObject* parent)
+~Session()
+id() int
-m_xwayland WXWayland*
-m_socket WSocket*
-m_settingManager
}
class WXWayland {
+create(WServer* server) void
+destroy(WServer* server) void
-surfaceList
-handle() QWXWayland*
}
class QWXWayland {
+get_xwm_connection() xcb_connection_t*
+destroy() void
}
class Treeland {
+quit() void
}
class TreelandPrivate {
+~TreelandPrivate()
-qmlEngine QQmlEngine*
-pluginTs
}
class QQuickWindow
class QQuickItem
class WOutputRenderWindow
class QSGRenderer
class WSeat
class QWindow
class QInputEvent
class WSocket
class ShellHandler
class WServer
class PersonalizationV1
WBufferRenderer --> QQuickItem : inherits
WBufferRenderer --> QQuickWindow : window()
WBufferRenderer --> WOutputRenderWindow : renderWindow()
WBufferRenderer --> QSGRenderer : uses in sources
RhiManager --> QQuickWindow : owner
RhiManager --> QSGRenderer : renderer
Helper --> ShellHandler : owns
Helper --> WSocket : manages
Helper --> WXWayland : factory
Session --> WXWayland : m_xwayland
Session --> Helper : uses instance
Session --> WSocket : m_socket
WXWayland --> QWXWayland : handle
WXWayland --> WServer : create, destroy
QWXWayland --> wlr_xwayland : wraps
Treeland --> TreelandPrivate : holds d_ptr
TreelandPrivate --> QQmlEngine : qmlEngine
TreelandPrivate --> UserModel : singleton
Helper ..> Treeland : requestQuit signal
QQuickWindow ..> WBufferRenderer : sceneGraphInvalidated signal
QQuickWindow ..> RhiManager : sceneGraphInvalidated signal
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey - I've found 2 issues, and left some high level feedback:
- The direct
connect(window(), &QQuickWindow::sceneGraphInvalidated, ...)inWBufferRenderer's constructor riskswindow()being null at construction time (as your own TODO notes); consider wiring this up viawindowChanged,componentComplete, or a lazy one-shot connection when a non-null window is first available. - Unconditionally calling
std::terminate()at the end ofTreeland::quit()forces an abnormal shutdown and bypasses normal object destruction; it might be safer to guard this behind a debug flag or dedicated emergency path so that non-debug quits can still follow a regular teardown sequence.
Prompt for AI Agents
Please address the comments from this code review: ## Overall Comments - The direct `connect(window(), &QQuickWindow::sceneGraphInvalidated, ...)` in `WBufferRenderer`'s constructor risks `window()` being null at construction time (as your own TODO notes); consider wiring this up via `windowChanged`, `componentComplete`, or a lazy one-shot connection when a non-null window is first available. - Unconditionally calling `std::terminate()` at the end of `Treeland::quit()` forces an abnormal shutdown and bypasses normal object destruction; it might be safer to guard this behind a debug flag or dedicated emergency path so that non-debug quits can still follow a regular teardown sequence. ## Individual Comments ### Comment 1 <location> `waylib/src/server/qtquick/private/wbufferrenderer.cpp:90-91` </location> <code_context> - + // ensure graphical resources are deleted when scene graph is invalidated + // TODO: WBufferRenderer created through WOutputViewport has window() == nullptr? + connect(window(), &QQuickWindow::sceneGraphInvalidated, + this, &WBufferRenderer::invalidateSceneGraph, + Qt::DirectConnection); } </code_context> <issue_to_address> **issue:** Connecting to window() once in the ctor can miss later window changes or operate on nullptr. This assumes the item already has a permanent window. For items constructed before being added to a window, `connect(window(), ...)` is effectively a no-op, so `sceneGraphInvalidated` will never be received. If the item later moves to a different window, the connection will still target the original one. Instead, listen to `QQuickItem::windowChanged` and (dis)connect `sceneGraphInvalidated` when the window becomes available or changes. </issue_to_address> ### Comment 2 <location> `src/core/treeland.cpp:106-107` </location> <code_context> it->second->deleteLater(); pluginTs.erase(it++); } + delete qmlEngine->singletonInstance<UserModel *>("Treeland", "UserModel"); + qmlEngine->clearSingletons(); } </code_context> <issue_to_address> **issue (bug_risk):** Manually deleting a QML singleton instance is likely a double-free; ownership belongs to the engine. `singletonInstance<T>()` objects are owned and destroyed by the QML engine (e.g. during teardown or `clearSingletons()`). Deleting this pointer here and then calling `clearSingletons()` can cause a double delete or other lifetime issues if the engine still tracks it. Please avoid deleting it directly; rely on `clearSingletons()` or change the singleton registration so the engine does not own the instance if you need manual control. </issue_to_address>
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Contributor
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Improve shutdown/termination stability by explicitly releasing Qt Quick scene graph resources on invalidation, fixing XWayland destruction ownership, and adjusting Treeland teardown ordering (with a temporary emergency quit key).
中文:通过在 scene graph 失效时显式释放 Qt Quick 图形资源、修正 XWayland 的销毁所有权与调用路径,并调整 Treeland 的销毁顺序,来提升退出/销毁阶段的稳定性(并临时加入紧急退出快捷键)。
Changes:
- Release Qt Quick renderers/texture providers on
QQuickWindow::sceneGraphInvalidatedand fixWBufferRendererwindow lookup. - Expose and call XWayland
destroy()from the compositor side; adjust Session/XWayland cleanup path. - Adjust Helper/Treeland destruction timing and add a temporary Meta+F12 emergency quit path.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
waylib/src/server/qtquick/private/wrenderbuffernode.cpp |
Attempt to destroy RhiManager on scene graph invalidation to release QSGRenderer earlier. |
waylib/src/server/qtquick/private/wbufferrenderer_p.h |
Use window() instead of parent() to obtain the WOutputRenderWindow. |
waylib/src/server/qtquick/private/wbufferrenderer.cpp |
Connect invalidation handler and free texture providers/renderers on scene graph invalidation. |
waylib/src/server/protocols/wxwayland.cpp |
Explicitly destroy underlying XWayland handle during teardown. |
qwlroots/src/types/qwxwayland.h |
Expose qw_xwayland::destroy() binding. |
src/session/session.cpp |
Make Session teardown resilient when Helper is already destroyed. |
src/seat/helper.h |
Remove Helper::removeXWayland declaration (cleanup now via ShellHandler). |
src/seat/helper.cpp |
Clear Helper::m_instance earlier during destruction; add Meta+F12 quit signal emission. |
src/core/treeland.cpp |
Adjust singleton teardown and force-terminate on quit. |
zccrs
zccrs
requested changes
Feb 4, 2026
@misaka18931
misaka18931
force-pushed
the
owoqwq
branch
6 times, most recently
from
February 4, 2026 08:06
3847114 to
c05e744
Compare
TAG Bot
New tag: 0.8.3
DISTRIBUTION: unstable
Suggest: synchronizing this PR through rebase #733
Changes: - RhiManager: destruct on `QQuickWindow::sceneGraphInvalidated`: RhiManager holds a `QSGRenderer*` which must be released before scene graph invalidation. RhiManager itself cannot function without that renderer. - WBufferRenderer: instance method `outputWindow()` should use `window()` instead of `parent()`, as its parents are generally not of class `WOutputRenderWindow`. - WBufferRenderer: release texture providers and renderers upon scene graph invalidation. - WBufferRenderer: explicitly connect slot `invalidateSceneGraph` to `QQuickWindow::sceneGraphInvalidated`, as Qt auto discovery can fail in practice.
`wlr_xwayland` is a compositor owned object, and thus should be destroyed compositor-side.
WXWayland instances are managed by `ShellHandler`, and should be deleted by it if possible. Otherwise, let WServer clean up. The underlying wlr_xwayland object for XWayland instances must be explicitly destroyed through wlr_xwayland_destroy, to clean up wayland event listensers of wlr_xwayland_shell_v1.
`Helper::m_instance` should be cleared immidiately when `Helper` begin destruction. Destroy UserModel before Helper to clean user sessions and surfaces, for their destruction often requires assistence of Helper. Destroy Helper before QmlEngine, as `SurfaceWrapper::createNewOrClose` requires a functional engine. Miscellaneous: cleaned unimplemented instance method `Helper::removeXWayland`.
Bring back the hard-coded Meta+F12 to crash Treeland, only for debug builds. Remapped the default normal quitting keybind to Meta+Shift+Q
@misaka18931
misaka18931
force-pushed
the
owoqwq
branch
from
February 4, 2026 08:19
c05e744 to
c863fea
Compare
zccrs
zccrs
requested changes
Feb 4, 2026
}
m_rhi.reset();
// Destructor for RhiManager runs in the GUI thread
deleteLater();
Member
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
算了,为了避免考虑 QQuickWindow::sceneGraphInitialized 信号,这里总归要销毁它,既然如此,这个机制做到 DataManagerBase 里去,在 sceneGraphInvalidated 时先把 this->setParent(nullptr); 然后直接 delete this; 即让这个对象在sceneGraph无效之前销毁。
Member
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
另外,在 DataManagerBase 的构造函数里断言一下 isSceneGraphInitialized
// Renderer of source is delay initialized in ensureRenderer. It might be null here.
if (s.renderer)
s.renderer->deleteLater();
delete s.renderer;
Member
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s.renderer = nullptr
m_sourceList.clear();
}
void WBufferRenderer::removeSource(int index)
Member
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
改名叫 destroySource,叫remove不合适,因为这个source还在list里,只是销毁了一些数据,没有从list里移除
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
fix: waylib: release renderers before scene graph invalidation
Changes:
QQuickWindow::sceneGraphInvalidated:RhiManager holds a
QSGRenderer*which must be released before scenegraph invalidation. RhiManager itself cannot function without that
renderer.
outputWindow()should usewindow()instead ofparent(), as its parents are generally not ofclass
WOutputRenderWindow.graph invalidation.
invalidateSceneGraphtoQQuickWindow::sceneGraphInvalidated, as Qt auto discovery can failin practice.
fix: qwlroots: expose qw_xwayland::destroy()
wlr_xwaylandis a compositor owned object, and thus should bedestroyed compositor-side.
fix: SessionManager: correct destruction routine of WXWayland instances
WXWayland instances are managed by
ShellHandler, and should be deletedby it if possible. Otherwise, let WServer clean up.
The underlying wlr_xwayland object for XWayland instances must be
explicitly destroyed through wlr_xwayland_destroy, to clean up wayland
event listensers of wlr_xwayland_shell_v1.
fix: Treeland destruction timing
Helper::m_instanceshould be cleared immidiately whenHelperbegindestruction.
Destroy UserModel before Helper to clean user sessions and surfaces,
for their destruction often requires assistence of Helper.
Destroy Helper before QmlEngine, as
SurfaceWrapper::createNewOrCloserequires a functional engine.
Miscellaneous: cleaned unimplemented instance method
Helper::removeXWayland.fix: temp: emergency restart key
Bring back the hard-coded Meta+F12 to crash Treeland, for debug purpose.
Currently treeland & ddm cannot handle quitting qwq.
Even a successful normal termination of Treeland leave the outputs with
the last rendered frame.
This should be reverted after a rework of the termination sequence.
Summary by Sourcery
Ensure Treeland and its Wayland/XWayland integrations shut down cleanly by fixing destruction order, resource release, and adding a temporary emergency restart shortcut.
New Features:
Bug Fixes:
Enhancements: