Skip to content
GitLab
Projects Groups Snippets
  • /
  • Help
    • Help
    • Support
    • Community forum
    • Submit feedback
    • Contribute to GitLab
  • Register
  • Sign in
  • G GLib
  • Project information
    • Project information
    • Activity
    • Labels
    • Members
  • Repository
    • Repository
    • Files
    • Commits
    • Branches
    • Tags
    • Contributors
    • Graph
    • Compare
  • Issues 853
    • Issues 853
    • List
    • Boards
    • Service Desk
    • Milestones
  • Merge requests 54
    • Merge requests 54
  • CI/CD
    • CI/CD
    • Pipelines
    • Jobs
    • Schedules
  • Deployments
    • Deployments
    • Releases
  • Packages and registries
    • Packages and registries
    • Container Registry
  • Analytics
    • Analytics
    • Value stream
    • CI/CD
    • Repository
  • Wiki
    • Wiki
  • Snippets
    • Snippets
  • Activity
  • Graph
  • Create a new issue
  • Jobs
  • Commits
  • Issue Boards
Collapse sidebar
  • GNOMEGNOME
  • GLib
  • Merge requests
  • !3291

gdbusconnection: Fix double unref on timeout/cancel sending a message

  • Review changes

  • Download
  • Email patches
  • Plain diff
Open Philip Withnall requested to merge pwithnall/glib:1264-gdbus-double-unref into main Feb 22, 2023
  • Overview 7
  • Commits 10
  • Pipelines 2
  • Changes 2

This appears to fix an intermittent failure seen when sending a D-Bus message with either of a cancellable or a timeout set.

In particular, I can reliably reproduce it with:

meson test gdbus-test-codegen-min-required-2-64 --repeat 10000

It can be caught easily with asan when reproduced. Tracking down the location of the refcount mismatch was a little tricky, but was simplified by replacing a load of g_object_ref (message) calls with g_dbus_message_copy (message, NULL) to switch GDBusMessage handling to using copy semantics. This allowed asan to home in on where the refcount mismatch was happening.

The problem was that send_message_data_deliver_error() takes ownership of the GTask passed to it, but the send_message_with_replace_cancelled_idle_cb() and send_message_with_reply_timeout_cb() functions which were calling it, were not passing in a strong reference as they should have.

Another approach to fixing this would have been to change the transfer semantics of send_message_data_deliver_error() so it was (transfer none) on its GTask. That would probably have resulted in cleaner code, but would have been a lot harder to verify/review the fix, and easier to inadvertently introduce new bugs. Edit: I found more bugs in this area, so eventually did change the transfer semantics of send_message_data_deliver_error(), in commit f01adb3d.

The fact that the bug was only triggered by the cancellation and timeout callbacks explains why it was intermittent: these code paths are typically never hit, but the timeout path may sometimes be hit on a very slow test run.

Signed-off-by: Philip Withnall pwithnall@endlessos.org

Fixes: #1264

Closes #1264

Edited Feb 23, 2023 by Philip Withnall
Assignee
Assign to
Reviewers
Request review from
Time tracking
Source branch: 1264-gdbus-double-unref