Revert "fix(dfm-io): drain GLib deferred callbacks after g_file_enumerate_children"#336
Conversation
…rate_children" This reverts commit 6013dea.
Reviewer's guide (collapsed on small PRs)Reviewer's GuideThis PR reverts a previous fix that drained GLib deferred callbacks after g_file_enumerate_children(), removing the explicit GMainContext iteration from the enumerator creation path. Sequence diagram for DEnumeratorPrivate createEnumerator without GLib main context drainingsequenceDiagram
participant DEnumeratorPrivate
participant GLib
DEnumeratorPrivate->>GLib: g_file_enumerate_children
GLib-->>DEnumeratorPrivate: GFileEnumerator* result
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:
- Since this reverts a relatively detailed leak workaround, consider expanding the commit message to capture why the GLib idle-drain logic is no longer needed or is problematic, so future maintainers don’t reintroduce similar code without context.
- If the original issue (GWeakRef leak when no GLib main loop runs on the QThread) is now handled elsewhere, it might help to add a brief pointer in the surrounding code or a code comment to indicate where that responsibility lives.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Since this reverts a relatively detailed leak workaround, consider expanding the commit message to capture why the GLib idle-drain logic is no longer needed or is problematic, so future maintainers don’t reintroduce similar code without context.
- If the original issue (GWeakRef leak when no GLib main loop runs on the QThread) is now handled elsewhere, it might help to add a brief pointer in the surrounding code or a code comment to indicate where that responsibility lives.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Johnson-zs, liyigang1 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 |
deepin pr auto review★ 总体评分:60分■ 【总体评价】
■ 【详细分析】
■ 【改进建议代码示例】 @@ -99,6 +99,19 @@ bool DEnumeratorPrivate::createEnumerator(const QUrl &url, QPointer<DEnumeratorP
enumLinks ? G_FILE_QUERY_INFO_NONE : G_FILE_QUERY_INFO_NOFOLLOW_SYMLINKS,
cancellable,
&gerror);
+
+ // g_file_enumerate_children internally creates and destroys temporary
+ // GDBusProxy instances (mount tracker proxy, daemon proxy). During proxy
+ // finalization, g_dbus_connection_signal_unsubscribe defers the
+ // user_data_free_func (weak_ref_free) to the thread-default GMainContext
+ // via call_destroy_notify() — a g_idle_source. Process it now while we
+ // are on the same thread, otherwise the GWeakRef (8 bytes from
+ // gdbusproxy.c:112) leaks when no GLib main loop runs on this QThread.
+ // See: glib2.0/gio/gdbusconnection.c:signal_subscriber_unref → call_destroy_notify
+ GMainContext *ctx = g_main_context_get_thread_default();
+ if (ctx) {
+ while (g_main_context_pending(ctx))
+ g_main_context_iteration(ctx, FALSE);
+ }
if (!me) {
// Clean up the enumerator if it was created but the object is no longer valid
if (genumerator) { |
|
@Johnson-zs: The following test failed, say
Full PR test history. Your PR dashboard. 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. I understand the commands that are listed here. |
4815828
into
linuxdeepin:semantic-search
This reverts commit 6013dea.
Summary by Sourcery
Revert the GLib main-context draining after g_file_enumerate_children to restore the previous DEnumerator behavior.