Skip to content
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

fix: fix crash when closing folder list widget #2535

Merged
merged 1 commit into from
Jan 2, 2025

Conversation

Johnson-zs
Copy link
Contributor

The crash occurred when the folder list widget was being destroyed while CrumbBar was already deleted. This fix includes:

  1. Add weak pointer to CrumbBar to avoid accessing deleted object
  2. Move popup visible state update to folder list widget's hidden signal
  3. Clean up folder list widget in UrlPushButton's destructor

Log: This ensures proper cleanup of resources and prevents accessing deleted objects.

The crash occurred when the folder list widget was being destroyed while
CrumbBar was already deleted. This fix includes:

1. Add weak pointer to CrumbBar to avoid accessing deleted object
2. Move popup visible state update to folder list widget's hidden signal
3. Clean up folder list widget in UrlPushButton's destructor

Log: This ensures proper cleanup of resources and prevents accessing deleted objects.
@deepin-ci-robot
Copy link

deepin pr auto review

代码审查意见:

  1. 内存管理

    • UrlPushButtonPrivate 析构函数中,folderListWidget 被隐藏并删除。这是一个好的做法,但需要确保 folderListWidget 的指针在删除后不再被使用,以避免潜在的悬空指针问题。
  2. 代码重复

    • onSelectSubDirs 函数中,folderListWidget 的隐藏和 crumbBar 的设置重复出现。可以考虑将这部分逻辑提取到一个单独的函数中,以减少代码重复。
  3. 弱引用的使用

    • onSelectSubDirs 函数中,使用 QPointer 保存 crumbBar 的弱引用是一个好的做法,可以避免潜在的悬空指针问题。但是,需要确保在 crumbBar 被删除后,相关的逻辑能够正确处理。
  4. 事件循环的使用

    • onSelectSubDirs 函数中,使用 QEventLoop 来等待 FolderListWidget 的隐藏事件。这是一个有效的方法,但需要确保 QEventLoop 不会阻塞主线程,影响用户体验。
  5. 注释

    • onSelectSubDirs 函数中,注释应该更详细地解释代码的逻辑和目的,特别是对于非堆叠和堆叠目录的显示逻辑。
  6. 资源泄露

    • 如果 folderListWidgetonSelectSubDirs 函数中被创建,但没有在析构函数中删除,可能会导致资源泄露。需要确保 folderListWidget 在适当的时候被删除。
  7. 连接的清理

    • UrlPushButtonPrivate 析构函数中,应该清理所有与 folderListWidgetcrumbBar 的连接,以避免潜在的内存泄漏。
  8. 错误处理

    • onSelectSubDirs 函数中,如果 crumbDatas 为空,应该有一个明确的错误处理逻辑,而不是简单地返回。

综上所述,代码在内存管理、代码重复、弱引用使用、事件循环使用、注释、资源泄露、连接清理和错误处理等方面存在一些改进空间。

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Johnson-zs, wyu71

The full list of commands accepted by this bot can be found here.

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

@Johnson-zs
Copy link
Contributor Author

/forcemerge

@deepin-bot
Copy link
Contributor

deepin-bot bot commented Jan 2, 2025

This pr force merged! (status: blocked)

@deepin-bot deepin-bot bot merged commit 97fc154 into linuxdeepin:master Jan 2, 2025
20 checks passed
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