fix: destroy wayland proxy before cleanup to prevent use-after-free crash#1621
fix: destroy wayland proxy before cleanup to prevent use-after-free crash#1621LFRon wants to merge 1 commit into
Conversation
…rash Call destroy() on the treeland_foreign_toplevel_handle_v1 proxy before emitting handlerIsDeleted() to stop the compositor from sending further events to a handle scheduled for C++ destruction. Without this, pending state events could arrive after the ForeignToplevelHandle object is freed by QScopedPointer, causing a crash in m_states.append() when activating applications from the system tray.
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: LFRon 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 |
|
Hi @LFRon. Thanks for your PR. 😃 |
|
Hi @LFRon. Thanks for your PR. I'm waiting for a linuxdeepin member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
Reviewer's guide (collapsed on small PRs)Reviewer's GuideEnsures the Wayland treeland_foreign_toplevel_handle_v1 proxy is explicitly destroyed before the corresponding C++ ForeignToplevelHandle object is torn down, preventing compositor events from targeting a freed object and causing crashes when activating apps from the system tray. Sequence diagram for destroying treeland_foreign_toplevel_handle_v1 before C++ teardownsequenceDiagram
participant Compositor
participant ForeignToplevelHandle
participant DockTaskManager
Compositor->>ForeignToplevelHandle: treeland_foreign_toplevel_handle_v1_closed
ForeignToplevelHandle->>ForeignToplevelHandle: destroy
ForeignToplevelHandle->>DockTaskManager: handlerIsDeleted
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Consider whether
treeland_foreign_toplevel_handle_v1_closedcan be invoked multiple times and, if so, whetherdestroy()is idempotent or should guard against double calls to avoid double-destroying the proxy. - It may be worth confirming that calling
destroy()before emittinghandlerIsDeleted()cannot trigger re-entrant callbacks back into this object (e.g., further Wayland events) that assume state already cleaned up, and if so, adding defensive checks or state flags.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider whether `treeland_foreign_toplevel_handle_v1_closed` can be invoked multiple times and, if so, whether `destroy()` is idempotent or should guard against double calls to avoid double-destroying the proxy.
- It may be worth confirming that calling `destroy()` before emitting `handlerIsDeleted()` cannot trigger re-entrant callbacks back into this object (e.g., further Wayland events) that assume state already cleaned up, and if so, adding defensive checks or state flags.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
在treeland渲染器下, 点击系统托盘(dde-tray-loader)中的应用图标调出应用主窗口时会引发整个dde-shell (dock栏)崩溃, 该PR用于修复这个问题
Call destroy() on the treeland_foreign_toplevel_handle_v1 proxy before emitting handlerIsDeleted() to stop the compositor from sending further events to a handle scheduled for C++ destruction. Without this, pending state events could arrive after the ForeignToplevelHandle object is freed by QScopedPointer, causing a crash in m_states.append() when activating applications from the system tray.
Summary by Sourcery
Bug Fixes: