Skip to content

fix: handle surface null case in setOutputs#608

Merged
zccrs merged 1 commit intolinuxdeepin:develop/splash-screenfrom
wineee:fix-output
Oct 29, 2025
Merged

fix: handle surface null case in setOutputs#608
zccrs merged 1 commit intolinuxdeepin:develop/splash-screenfrom
wineee:fix-output

Conversation

@wineee
Copy link
Member

@wineee wineee commented Oct 29, 2025

  1. Added null check for surface() in setOutputs method to prevent
    crashes when surface is null
  2. Added prelaunch outputs storage to preserve output settings during
    surface initialization phase
  3. Applied prelaunch outputs when converting to normal surface to ensure
    proper output assignment
  4. This fixes a race condition where WSurface could be null while
    WXWaylandSurface still exists

Influence:

  1. Test surface creation and output assignment during prelaunch phase
  2. Verify that surfaces properly inherit output settings when converted
    to normal surfaces
  3. Test edge cases where surfaces might be destroyed while operations
    are pending
  4. Validate that no crashes occur when setting outputs on null surfaces

fix: 修复 setOutputs 中 surface 为 null 的情况处理

  1. 在 setOutputs 方法中添加了对 surface() 的空检查,防止 surface 为 null
    时崩溃
  2. 添加了预启动输出存储功能,在 surface 初始化阶段保留输出设置
  3. 在转换为普通 surface 时应用预启动输出,确保正确的输出分配
  4. 修复了 WSurface 可能为 null 而 WXWaylandSurface 仍存在的竞态条件

Influence:

  1. 测试预启动阶段的 surface 创建和输出分配
  2. 验证 surface 转换为普通 surface 时是否正确继承输出设置
  3. 测试 surface 可能被销毁而操作仍在挂起的边缘情况
  4. 确认在 null surface 上设置输出时不会发生崩溃

Summary by Sourcery

Add null-safe handling and prelaunch output caching in SurfaceWrapper to prevent crashes and ensure correct output assignment when surfaces initialize.

Bug Fixes:

  • Prevent crashes in setOutputs when surface() is null by adding a null check.
  • Fix race condition where outputs could be applied before the surface was initialized.

Enhancements:

  • Cache output settings during the prelaunch phase and apply them upon conversion to a normal surface.

1. Added null check for surface() in setOutputs method to prevent
crashes when surface is null
2. Added prelaunch outputs storage to preserve output settings during
surface initialization phase
3. Applied prelaunch outputs when converting to normal surface to ensure
proper output assignment
4. This fixes a race condition where WSurface could be null while
WXWaylandSurface still exists

Influence:
1. Test surface creation and output assignment during prelaunch phase
2. Verify that surfaces properly inherit output settings when converted
to normal surfaces
3. Test edge cases where surfaces might be destroyed while operations
are pending
4. Validate that no crashes occur when setting outputs on null surfaces

fix: 修复 setOutputs 中 surface 为 null 的情况处理

1. 在 setOutputs 方法中添加了对 surface() 的空检查,防止 surface 为 null
时崩溃
2. 添加了预启动输出存储功能,在 surface 初始化阶段保留输出设置
3. 在转换为普通 surface 时应用预启动输出,确保正确的输出分配
4. 修复了 WSurface 可能为 null 而 WXWaylandSurface 仍存在的竞态条件

Influence:
1. 测试预启动阶段的 surface 创建和输出分配
2. 验证 surface 转换为普通 surface 时是否正确继承输出设置
3. 测试 surface 可能被销毁而操作仍在挂起的边缘情况
4. 确认在 null surface 上设置输出时不会发生崩溃
@sourcery-ai
Copy link

sourcery-ai bot commented Oct 29, 2025

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

This PR refactors the output assignment logic to safely buffer outputs during the prelaunch phase and apply them once the surface is fully initialized, while adding a guard to prevent crashes when surface() is null, fixing a race condition between WXWaylandSurface and WSurface.

Sequence diagram for output assignment during prelaunch and surface initialization

sequenceDiagram
    participant Client
    participant SurfaceWrapper
    participant WSurface
    participant WOutput

    Client->>SurfaceWrapper: setOutputs(outputs) (prelaunch phase)
    SurfaceWrapper->>SurfaceWrapper: Store outputs in m_prelaunchOutputs
    Note over SurfaceWrapper: surface() is null, outputs buffered
    Client->>SurfaceWrapper: convertToNormalSurface()
    SurfaceWrapper->>SurfaceWrapper: setup()
    SurfaceWrapper->>SurfaceWrapper: Apply m_prelaunchOutputs via setOutputs()
    SurfaceWrapper->>WSurface: setOutputs(outputs)
    WSurface->>WOutput: Assign outputs
    SurfaceWrapper->>SurfaceWrapper: Clear m_prelaunchOutputs
Loading

Class diagram for updated SurfaceWrapper output handling

classDiagram
    class SurfaceWrapper {
        +QList<WOutput*> m_prelaunchOutputs
        +void setOutputs(const QList<WOutput*>& outputs)
        +void convertToNormalSurface(WToplevelSurface* shellSurface, Type type)
    }
    class WSurface {
        +QList<WOutput*> outputs()
    }
    class WOutput
    SurfaceWrapper --> WSurface : surface()
    SurfaceWrapper --> WOutput : m_prelaunchOutputs
    SurfaceWrapper ..> WToplevelSurface : convertToNormalSurface()
Loading

File-Level Changes

Change Details Files
Pre-launch outputs are buffered and applied when converting to a normal surface
  • Added m_prelaunchOutputs member to store outputs during prelaunch
  • In setOutputs: if type is Undetermined, save outputs in m_prelaunchOutputs and return early
  • In convertToNormalSurface: detect buffered outputs, call setOutputs, then clear m_prelaunchOutputs
src/surface/surfacewrapper.cpp
src/surface/surfacewrapper.h
Prevent crashes by guarding against null surface() in setOutputs
  • Inserted a null check for surface() before proceeding with output assignment and logged a debug message
  • Removed the Q_ASSERT(surface()) in favor of the null guard
src/surface/surfacewrapper.cpp

Possibly linked issues

  • fix: crash when popup window show #24: PR adds null check for surface in setOutputs and handles prelaunch outputs, fixing the WSurface::outputs() crash in issue fix: crash when popup window show #24.
  • #issue_provided_in_context: The PR fixes a null surface race condition in setOutputs that could cause SurfaceWrapper destructor crashes, directly addressing the issue.
  • #[Bug]: destroy_buffer崩溃: The PR resolves a race condition where WSurface can be null, preventing the destroy_buffer segmentation fault by ensuring valid surface states during output management.

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey there - I've reviewed your changes and they look great!


Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@wineee wineee mentioned this pull request Oct 29, 2025
16 tasks
@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: wineee, zccrs

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

@zccrs zccrs merged commit 8c7eb86 into linuxdeepin:develop/splash-screen Oct 29, 2025
6 of 10 checks passed
@wineee wineee deleted the fix-output branch October 29, 2025 07:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants