fix: fix occasional crash in treeland under layer shell surface#1577
Merged
deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom Apr 30, 2026
Merged
fix: fix occasional crash in treeland under layer shell surface#1577deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
Conversation
18202781743
requested review from
BLumia,
fly602,
mhduiy,
robertkill,
wjyrich and
yixinshark
Reviewer's GuideRefactors QWaylandLayerShellSurface to debounce Wayland surface commits and layer-shell surface recreation on screen changes, avoiding use-after-free crashes by scheduling operations on the object itself, tracking wl_output, and guarding against null window/surface. Sequence diagram for debounced Wayland surface commits via scheduleCommitsequenceDiagram
participant DLayerShellWindow
participant QWaylandLayerShellSurface
participant QtWaylandClient_QWaylandWindow as QWaylandWindow
participant QtWaylandClient_QWaylandSurface as QWaylandSurface
rect rgb(230,230,250)
DLayerShellWindow->>QWaylandLayerShellSurface: layerChanged
QWaylandLayerShellSurface->>QWaylandLayerShellSurface: set_layer
QWaylandLayerShellSurface->>QWaylandLayerShellSurface: scheduleCommit
end
rect rgb(230,230,250)
DLayerShellWindow->>QWaylandLayerShellSurface: exclusionZoneChanged
QWaylandLayerShellSurface->>QWaylandLayerShellSurface: set_exclusive_zone
QWaylandLayerShellSurface->>QWaylandLayerShellSurface: scheduleCommit
end
rect rgb(230,230,250)
DLayerShellWindow->>QWaylandLayerShellSurface: marginsChanged
QWaylandLayerShellSurface->>QWaylandLayerShellSurface: set_margin
QWaylandLayerShellSurface->>QWaylandLayerShellSurface: scheduleCommit
end
rect rgb(230,230,250)
DLayerShellWindow->>QWaylandLayerShellSurface: keyboardInteractivityChanged
QWaylandLayerShellSurface->>QWaylandLayerShellSurface: set_keyboard_interactivity
QWaylandLayerShellSurface->>QWaylandLayerShellSurface: scheduleCommit
end
rect rgb(230,230,250)
DLayerShellWindow->>QWaylandWindow: inputRegionChanged
QWaylandWindow->>QWaylandLayerShellSurface: applyInputRegion lambda
QWaylandLayerShellSurface->>QWaylandWindow: window.setMask
QWaylandLayerShellSurface->>QWaylandLayerShellSurface: scheduleCommit
end
rect rgb(220,255,220)
QWaylandLayerShellSurface->>QWaylandLayerShellSurface: scheduleCommit
alt first call
QWaylandLayerShellSurface->>QWaylandLayerShellSurface: m_commitScheduled = true
QWaylandLayerShellSurface-->>QWaylandLayerShellSurface: invokeMethod Qt QueuedConnection
else subsequent calls while m_commitScheduled is true
QWaylandLayerShellSurface-->>QWaylandLayerShellSurface: return without scheduling
end
end
rect rgb(220,240,255)
QWaylandLayerShellSurface-->>QWaylandLayerShellSurface: queued lambda executes
QWaylandLayerShellSurface->>QWaylandLayerShellSurface: m_commitScheduled = false
QWaylandLayerShellSurface->>QWaylandLayerShellSurface: commitWindowState
QWaylandLayerShellSurface->>QWaylandWindow: window()
alt window or waylandSurface is null
QWaylandLayerShellSurface-->>QWaylandLayerShellSurface: return, skip commit
else valid window and surface
QWaylandLayerShellSurface->>QWaylandSurface: commit
end
end
Sequence diagram for debounced layer shell surface recreation on screenChangedsequenceDiagram
participant QWindow as QWindow
participant QWaylandLayerShellSurface
participant QtWaylandClient_QWaylandWindow as QWaylandWindow
participant QtWaylandClient_QWaylandScreen as QWaylandScreen
participant wl_output as wl_output
rect rgb(230,230,250)
QWindow->>QWaylandLayerShellSurface: screenChanged
QWaylandLayerShellSurface->>QWaylandLayerShellSurface: scheduleRecreate
end
rect rgb(220,255,220)
QWaylandLayerShellSurface->>QWaylandWindow: window()
alt window missing or destroyed
QWaylandLayerShellSurface-->>QWaylandLayerShellSurface: return, no recreate
else window exists
QWaylandLayerShellSurface->>QWaylandWindow: window()
QWaylandWindow->>QWindow: screen()
QWindow->>QWaylandScreen: handle cast
alt waylandScreen is null
QWaylandLayerShellSurface-->>QWaylandLayerShellSurface: warning, return
else waylandScreen valid
QWaylandScreen->>wl_output: output
QWaylandLayerShellSurface->>QWaylandLayerShellSurface: compare output with m_output
alt output unchanged or m_recreateScheduled true
QWaylandLayerShellSurface-->>QWaylandLayerShellSurface: return
else new output and no recreate scheduled
QWaylandLayerShellSurface->>QWaylandLayerShellSurface: m_output = output
QWaylandLayerShellSurface->>QWaylandLayerShellSurface: m_recreateScheduled = true
QWaylandLayerShellSurface-->>QWaylandLayerShellSurface: invokeMethod Qt QueuedConnection
end
end
end
end
rect rgb(220,240,255)
QWaylandLayerShellSurface-->>QWaylandLayerShellSurface: queued recreate lambda executes
QWaylandLayerShellSurface->>QWaylandLayerShellSurface: m_recreateScheduled = false
QWaylandLayerShellSurface->>QWaylandWindow: window()
alt window destroyed before recreate
QWaylandLayerShellSurface-->>QWaylandLayerShellSurface: return, skip reset and reinit
else window still alive
QWaylandLayerShellSurface->>QWaylandWindow: reset
QWaylandLayerShellSurface-->>QWaylandWindow: invokeMethod reinit Qt QueuedConnection
QWaylandWindow-->>QWaylandWindow: reinit executes asynchronously
end
end
Class diagram for updated QWaylandLayerShellSurface debouncing and output trackingclassDiagram
class QWaylandLayerShellSurface {
+QWaylandLayerShellSurface(zwlr_layer_surface_v1* surface, DLayerShellWindow* dlayerShellWindow)
+void setWindowGeometry(QRect geometry)
+void calcAndSetRequestSize(QSize requestSize)
+bool anchorsSizeConflict()
+void trySetAnchorsAndSize()
+void commitWindowState()
+void scheduleCommit()
+void scheduleRecreate()
-DLayerShellWindow* m_dlayerShellWindow
-QSize m_pendingSize
-QSize m_requestSize
-wl_output* m_output
-bool m_configured
-bool m_commitScheduled
-bool m_recreateScheduled
}
class DLayerShellWindow {
+QtWaylandClient_QWaylandWindow* window()
+QtWaylandClient_QWaylandSurface* waylandSurface()
+int layer()
+QString scope()
+Qt::ScreenOrientation screenConfiguration()
+Qt::Edges anchors()
+int exclusionZone()
+int topMargin()
+int rightMargin()
+int bottomMargin()
+int leftMargin()
+QRegion inputRegion()
+int keyboardInteractivity()
+void reset()
+void reinit()
<<signals>> layerChanged
<<signals>> exclusionZoneChanged
<<signals>> marginsChanged
<<signals>> keyboardInteractivityChanged
<<signals>> inputRegionChanged
}
class QtWaylandClient_QWaylandShellSurface {
+void init(wl_surface* surface)
+void set_layer(int layer)
+void set_anchor(int anchors)
+void set_size(int width, int height)
+void set_margin(int top, int right, int bottom, int left)
+void set_exclusive_zone(int zone)
+void set_keyboard_interactivity(int mode)
}
class QtWaylandClient_QWaylandWindow {
+QWindow* window()
+QtWaylandClient_QWaylandSurface* waylandSurface()
+void reset()
+void reinit()
}
class QtWaylandClient_QWaylandSurface {
+void commit()
}
QWaylandLayerShellSurface --|> QtWaylandClient_QWaylandShellSurface
QWaylandLayerShellSurface ..> DLayerShellWindow : observes
QWaylandLayerShellSurface ..> QtWaylandClient_QWaylandWindow : uses window
QtWaylandClient_QWaylandWindow ..> QtWaylandClient_QWaylandSurface : owns
QtWaylandClient_QWaylandSurface <.. QWaylandLayerShellSurface : commitWindowState
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- In scheduleRecreate(), consider adding null checks for waylandWindow->window(), window()->screen(), and screen()->handle() before the dynamic_cast, as screen changes or teardown could temporarily leave these as nullptr and reintroduce a crash path.
- scheduleRecreate() updates m_output and sets m_recreateScheduled before the queued reset/reinit runs; if multiple screen changes occur quickly, later changes are ignored—if that’s not intentional, you may want to track the latest pending output and always recreate to the most recent one when the queued call executes.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In scheduleRecreate(), consider adding null checks for waylandWindow->window(), window()->screen(), and screen()->handle() before the dynamic_cast, as screen changes or teardown could temporarily leave these as nullptr and reintroduce a crash path.
- scheduleRecreate() updates m_output and sets m_recreateScheduled before the queued reset/reinit runs; if multiple screen changes occur quickly, later changes are ignored—if that’s not intentional, you may want to track the latest pending output and always recreate to the most recent one when the queued call executes.
## Individual Comments
### Comment 1
<location path="frame/layershell/qwaylandlayershellsurface.cpp" line_range="192" />
<code_context>
+ return;
+ }
+
+ auto waylandScreen = dynamic_cast<QtWaylandClient::QWaylandScreen *>(waylandWindow->window()->screen()->handle());
+ if (!waylandScreen) {
+ qCWarning(layershellsurface) << "failed to get screen for wayland";
</code_context>
<issue_to_address>
**issue (bug_risk):** Consider guarding against a null QWindow::screen() before accessing its handle in scheduleRecreate().
`window()->screen()` can briefly be null (for example during screen transitions), but we dereference it unconditionally here. Please add a null check before using `screen()->handle()` to avoid a crash in those cases.
</issue_to_address>
### Comment 2
<location path="frame/layershell/qwaylandlayershellsurface.cpp" line_range="162" />
<code_context>
trySetAnchorsAndSize();
}
+void QWaylandLayerShellSurface::commitWindowState()
+{
+ auto waylandWindow = window();
</code_context>
<issue_to_address>
**issue (complexity):** Consider extracting reusable helpers and small slots for screen/output lookup, window recreation, and commit flushing to reduce lambda nesting, duplication, and custom state handling while preserving the new batching/debouncing behaviour.
You can keep the new behaviour (batched commits, debounced recreation) while reducing nesting, duplication, and custom state by extracting a couple of small helpers and using member-function invokes instead of lambdas.
### 1. Reduce lambda nesting & duplication for recreation
The reset/reinit sequence and `QWaylandScreen` lookup are duplicated and spread across lambdas. Pull them into helpers so `scheduleRecreate()` reads like a small state machine:
```cpp
QtWaylandClient::QWaylandScreen *QWaylandLayerShellSurface::currentWaylandScreen() const
{
auto waylandWindow = window();
if (!waylandWindow || !waylandWindow->window())
return nullptr;
return dynamic_cast<QtWaylandClient::QWaylandScreen *>(
waylandWindow->window()->screen()->handle());
}
wl_output *QWaylandLayerShellSurface::currentOutput() const
{
if (auto screen = currentWaylandScreen())
return screen->output();
qCWarning(layershellsurface) << "failed to get screen for wayland";
return nullptr;
}
void QWaylandLayerShellSurface::recreateWindow()
{
auto waylandWindow = window();
if (!waylandWindow)
return;
waylandWindow->reset();
QMetaObject::invokeMethod(
waylandWindow,
&QtWaylandClient::QWaylandWindow::reinit,
Qt::QueuedConnection);
}
void QWaylandLayerShellSurface::scheduleRecreate()
{
auto output = currentOutput();
if (!output || output == m_output || m_recreateScheduled)
return;
m_output = output;
m_recreateScheduled = true;
QMetaObject::invokeMethod(
this,
[this]() {
m_recreateScheduled = false;
recreateWindow();
},
Qt::QueuedConnection);
}
```
This keeps:
- debouncing via `m_recreateScheduled`,
- avoiding redundant recreation on the same `wl_output`,
- async `reset()` → queued `reinit()`,
but removes the deeply nested lambdas and repeated null/type checks.
### 2. Simplify commit scheduling plumbing
You can keep the coalescing behaviour while avoiding an extra lambda and making the intent clearer by turning `commitWindowState()` into an invokable helper:
```cpp
void QWaylandLayerShellSurface::commitWindowState()
{
auto waylandWindow = window();
if (!waylandWindow || !waylandWindow->waylandSurface())
return;
waylandWindow->waylandSurface()->commit();
}
void QWaylandLayerShellSurface::scheduleCommit()
{
if (m_commitScheduled)
return;
m_commitScheduled = true;
QMetaObject::invokeMethod(
this,
[this]() {
m_commitScheduled = false;
commitWindowState();
},
Qt::QueuedConnection);
}
```
If you want to go one step further, you can remove the inner lambda too and use a small private slot:
```cpp
private slots:
void flushScheduledCommit();
void QWaylandLayerShellSurface::flushScheduledCommit()
{
m_commitScheduled = false;
commitWindowState();
}
void QWaylandLayerShellSurface::scheduleCommit()
{
if (m_commitScheduled)
return;
m_commitScheduled = true;
QMetaObject::invokeMethod(this,
&QWaylandLayerShellSurface::flushScheduledCommit,
Qt::QueuedConnection);
}
```
This preserves batching (only one commit per event-loop turn) but makes the control flow around commits and recreation easier to follow and test.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
TAG Bot New tag: 2.0.39 |
18202781743
force-pushed
the
treeland-adapter
branch
from
afd6ec2 to
125b3ac
Compare
mhduiy
approved these changes
Apr 29, 2026
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 18202781743, mhduiy The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
18202781743
force-pushed
the
treeland-adapter
branch
2 times, most recently
from
8d39da2 to
3d891f7
Compare
1. Replace direct screenChanged lambda that captured window pointer with member function scheduleRecreate 2. Add m_output tracking to prevent redundant recreate calls 3. Add scheduleCommit to coalesce multiple commit calls and delay them via QueuedConnection 4. Add m_commitScheduled and m_recreateScheduled flags to debounce operations 5. Fix marginsChanged signal handler missing explicit commit 6. This prevents race conditions when window gets destroyed during asynchronous reset/reinit cycle Influence: 1. Test layer shell surface creation with screen changes 2. Verify no crashes when rapidly changing screen configuration 3. Test margins, anchors, keyboard interactivity changes 4. Verify commit debouncing works correctly 5. Test edge cases with null waylandSurface or window 6. Test on treeland to confirm crash is fixed 修复: 修复在 treeland 下图层壳表面的偶发崩溃 1. 将直接捕获窗口指针的 screenChanged lambda 替换为成员函数 scheduleRecreate 2. 添加 m_output 跟踪以防止重复的 recreate 调用 3. 添加 scheduleCommit 合并多个 commit 调用并通过 QueuedConnection 延迟 执行 4. 添加 m_commitScheduled 和 m_recreateScheduled 标志以去抖操作 5. 修复 marginsChanged 信号处理程序缺少显式 commit 的问题 6. 这防止了在异步 reset/reinit 周期中窗口被销毁时的竞态条件 Influence: 1. 测试屏幕变化时的图层壳表面创建 2. 验证快速改变屏幕配置时无崩溃 3. 测试边距、锚点、键盘交互性变化 4. 验证 commit 去抖工作正常 5. 测试 waylandSurface 或窗口为 null 的边界情况 6. 在 treeland 上测试确认崩溃已修复
18202781743
force-pushed
the
treeland-adapter
branch
from
3d891f7 to
3bbe3ca
Compare
deepin pr auto reviewGit Diff 代码审查报告总体评价本次代码重构对
详细审查1. 语法与逻辑优点
改进建议
2. 代码质量优点
改进建议
3. 性能优化优点
改进建议
4. 安全性优点
改进建议
总结这次代码重构整体上是一个很好的改进，提高了代码的可读性、可维护性和性能。主要的改进点是将重复的代码逻辑提取为独立方法，并引入了延迟提交机制来优化性能。 建议的改进主要集中在：
这些改进将进一步提高代码的健壮性和可靠性。 |
Contributor
Author
|
/forcemerge |
|
This pr force merged! (status: blocked) |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
member function scheduleRecreate
via QueuedConnection
operations
asynchronous reset/reinit cycle
Influence:
修复: 修复在 treeland 下图层壳表面的偶发崩溃
scheduleRecreate
执行
Influence:
Summary by Sourcery
Debounce layer shell surface state updates and screen-change handling to avoid crashes when recreating windows on Treeland.
Bug Fixes:
Enhancements: