Skip to content

fix: safe iteration with removeAt/erase across codebase#1012

Open
deepin-wm wants to merge 1 commit into
linuxdeepin:masterfrom
deepin-wm:fix/iterator-erase-safety
Open

fix: safe iteration with removeAt/erase across codebase#1012
deepin-wm wants to merge 1 commit into
linuxdeepin:masterfrom
deepin-wm:fix/iterator-erase-safety

Conversation

@deepin-wm

Copy link
Copy Markdown
Contributor

Summary

Iterator/erase/remove safety fixes (P0+P1) to prevent use-after-destroy and iterator invalidation issues.

Changes

src/core/shellhandler.cpp — for+removeAt+break → indexOf+takeAt (8 places)

Replaced forward-iteration patterns that called removeAt(i) followed by break with a safer indexOf + takeAt approach. The original pattern was fragile: if the break were ever removed or the loop continued, the iterator would be invalidated.

waylib/src/server/kernel/wseat.cpp — removeAt(i--) → reverse iteration (2 places)

In doTouchNotifyCancel and doTouchFrame, replaced forward removeAt(i--) with reverse traversal (for (int i = size - 1; i >= 0; --i)). Reverse iteration is inherently safe with removeAt because removing an element only shifts indices below the current position.

waylib/src/server/kernel/wglobal.cpp — removeAt(i--)+--i → reverse iteration

In WWrapObject::safeDisconnect, replaced forward loop with removeAt(i) + manual --i reset with a clean reverse iteration loop. Same safety rationale as above.

waylib/src/server/qtquick/private/wbufferrenderer.cpp — add ordering constraint comment

Added a comment clarifying that destroySource(index) must be called before m_sourceList.removeAt(index) because destroySource accesses m_sourceList[index].

src/input/gestures.cpp — remove redundant deleteLater in destroyed callback (2 places)

Removed gesture->deleteLater() calls from unregisterSwipeGesture and unregisterHoldGesture. These callbacks are connected to QObject::destroyed, which fires during object destruction — calling deleteLater() on a being-destroyed object is redundant and potentially harmful.

Testing

  • Touch input (multi-touch cancel and frame) for correctness after removing touch points
  • Prelaunch splash creation, close, and timeout flows
  • Xdg toplevel and xwayland surface creation with appId resolution
  • Gesture unregister during active gesture recognition
  • safeDisconnect with multiple connections to same receiver

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Sorry @deepin-wm, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@wineee

wineee commented Jun 18, 2026

Copy link
Copy Markdown
Member

Code review

No issues found. Checked for bugs and CLAUDE.md compliance. The 5-file iterator/erase safety fix is well-reasoned:

  • Reverse iteration in wseat.cpp and wglobal.cpp eliminates removeAt(i--) / removeAt(i)+--i patterns that risk skipping elements
  • takeAt in shellhandler.cpp correctly preserves the removed value for destroyForSurface calls
  • Removing deleteLater() in gestures.cpp is correct since destroyed callbacks fire during object destruction

🤖 Generated with Claude Code

@deepin-wm deepin-wm force-pushed the fix/iterator-erase-safety branch from dfcdbda to 45f4121 Compare June 18, 2026 11:10
1. Replace removeAt(i--) with reverse iteration in wseat.cpp (2 places) for safe element removal during traversal
2. Replace removeAt(i--)+--i with reverse iteration in wglobal.cpp (1 place) for safe element removal during traversal
3. Add ordering constraint comment in wbufferrenderer.cpp to clarify destroySource must precede removeAt

Log: Fixed iterator invalidation bugs in container traversal with concurrent element removal

Influence:
1. Test touch input (multi-touch cancel and frame) for correctness after removing touch points
2. Test safeDisconnect with multiple connections to same receiver
3. Test WBufferRenderer source list destruction order

fix: 修复waylib模块中迭代器与removeAt并发使用的安全问题

1. 在wseat.cpp(2处)中将removeAt(i--)改为倒序遍历,确保遍历中安全移除元素
2. 在wglobal.cpp(1处)中将removeAt(i--)+--i改为倒序遍历,确保遍历中安全移除元素
3. 在wbufferrenderer.cpp中添加顺序约束注释,说明destroySource必须在removeAt之前调用

Log: 修复了容器遍历中并发移除元素时的迭代器失效问题

Influence:
1. 测试触摸输入(多点触控取消和帧处理)在移除触摸点后的正确性
2. 测试同一接收者多连接场景下的safeDisconnect
3. 测试WBufferRenderer源列表销毁顺序
@deepin-wm deepin-wm force-pushed the fix/iterator-erase-safety branch from 45f4121 to a8f0f3c Compare June 18, 2026 13:32
@deepin-ci-robot

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: deepin-wm, wineee

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

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