GFileMonitor deadlocks

[Balint Reczey]

[ Description ]

https://autopkgtest.ubuntu.com/packages/libx/libxmlb/groovy/amd64

https://objectstorage.prodstack4-5.canonical.com/v1/AUTH_77e2ada1e7a84929a74ba3b87153c0ac/autopkgtest-groovy/groovy/amd64/libx/libxmlb/20200803_212158_ebe4f@/log.gz :

...
(/usr/libexec/installed-tests/libxmlb/xb-self-test:1066): XbSilo-DEBUG: 21:16:46.741: /tmp/temp.xml changed, invalidating
Executing: libxmlb/libxmlb.test
Executing: libxmlb/libxmlb.test
...
Executing: libxmlb/libxmlb.test
Executing: libxmlb/libxmlb.test
Executing: libxmlb/libxmlb.test
Executing: libxmlb/libxmlb.test
Test timed out after 300 seconds
Executing: libxmlb/libxmlb.test
FAIL: libxmlb/libxmlb.test (Child process killed by signal 9)
SUMMARY: total=1; passed=0; skipped=0; failed=1; user=0.0s; system=0.0s; maxrss=5900
FAIL: libxmlb/libxmlb.test (Child process killed by signal 9)
...

The backtrace while the test is hanging looks like:

(gdb) bt
#0 syscall () at ../sysdeps/unix/sysv/linux/x86_64/syscall.S:38
#1 0x00007fc9150d57cc in g_mutex_lock_slowpath (mutex=mutex@entry=0x7fc9153a0318 <g.inotify_lock_lock>)
    at ../../../glib/gthread-posix.c:1458
#2 0x00007fc9150d6256 in g_mutex_lock (mutex=mutex@entry=0x7fc9153a0318 <g.inotify_lock_lock>) at ../../../glib/gthread-posix.c:1482
#3 0x00007fc915309de4 in _ih_sub_cancel (sub=0x55a566f5be10) at ../../../gio/inotify/inotify-helper.c:121
#4 0x00007fc91530a06e in g_inotify_file_monitor_cancel (monitor=0x55a566f501a0) at ../../../gio/inotify/ginotifyfilemonitor.c:75
#5 0x00007fc91523680b in g_file_monitor_cancel (monitor=0x55a566f501a0) at ../../../gio/gfilemonitor.c:241
#6 g_file_monitor_cancel (monitor=0x55a566f501a0) at ../../../gio/gfilemonitor.c:235
#7 0x00007fc915236890 in g_file_monitor_dispose (object=0x55a566f501a0) at ../../../gio/gfilemonitor.c:123
#8 0x00007fc9152fe8b3 in g_local_file_monitor_dispose (object=0x55a566f501a0) at ../../../gio/glocalfilemonitor.c:814
#9 0x00007fc915179c93 in g_object_unref (_object=<optimized out>) at ../../../gobject/gobject.c:3461
#10 g_object_unref (_object=0x55a566f501a0) at ../../../gobject/gobject.c:3391
#11 0x000055a5666dce9d in xb_silo_file_monitor_item_free (item=0x55a566f53810) at ../src/xb-silo.c:1220
#12 0x00007fc915075172 in g_hash_table_remove_all_nodes (hash_table=hash_table@entry=0x55a566f62b60, notify=notify@entry=1,
    destruction=destruction@entry=1) at ../../../glib/ghash.c:706
#13 0x00007fc915076533 in g_hash_table_remove_all_nodes (destruction=1, notify=1, hash_table=0x55a566f62b60)
    at ../../../glib/ghash.c:1461
#14 g_hash_table_unref (hash_table=0x55a566f62b60) at ../../../glib/ghash.c:1461
#15 0x000055a5666dd445 in xb_silo_finalize (obj=0x55a566f47a00) at ../src/xb-silo.c:1320
#16 0x00007fc915179d0e in g_object_unref (_object=<optimized out>) at ../../../gobject/gobject.c:3499
#17 g_object_unref (_object=0x55a566f47a00) at ../../../gobject/gobject.c:3391
#18 0x000055a5666c4864 in xb_builder_finalize (obj=0x55a566f46da0) at ../src/xb-builder.c:1027
#19 0x00007fc915179d0e in g_object_unref (_object=<optimized out>) at ../../../gobject/gobject.c:3499
#20 g_object_unref (_object=0x55a566f46da0) at ../../../gobject/gobject.c:3391
#21 0x000055a5666db999 in glib_autoptr_clear_GObject (_ptr=0x55a566f46da0) at /usr/include/glib-2.0/gobject/gobject-autocleanups.h:27
#22 glib_autoptr_clear_XbBuilder (_ptr=0x55a566f46da0) at ../src/xb-builder.h:20
#23 glib_autoptr_cleanup_XbBuilder (_ptr=<synthetic pointer>) at ../src/xb-builder.h:20
#24 xb_builder_ensure_watch_source_func () at ../src/xb-self-test.c:501
#25 0x00007fc9150b13ce in test_case_run (tc=0x55a566f43b00) at ../../../glib/gtestutils.c:2633
#26 g_test_run_suite_internal (suite=suite@entry=0x55a566f42440, path=path@entry=0x0) at ../../../glib/gtestutils.c:2721
#27 0x00007fc9150b1174 in g_test_run_suite_internal (suite=suite@entry=0x55a566f42420, path=path@entry=0x0)
    at ../../../glib/gtestutils.c:2733
#28 0x00007fc9150b18ba in g_test_run_suite (suite=0x55a566f42420) at ../../../glib/gtestutils.c:2808
#29 0x00007fc9150b18d5 in g_test_run () at ../../../glib/gtestutils.c:2043
#30 0x000055a5666c38bb in main (argc=<optimized out>, argv=<optimized out>) at ../src/xb-self-test.c:2423
(gdb)

[ Fix ]

This is a hang when disposing the GFileMonitor.

See https://gitlab.gnome.org/GNOME/glib/-/issues/1941 for more details.

The fix is to explicitly cancel the GFileMonitor when when unreffing it.

[ QA ]

We don't have a testcase for the bug in real usage. It's quite easy to check in autopkgtest though. Check the autopkgtests stop being flaky/racy with the new version.

[ What could go wrong ]

We're cancelling the file monitor explicitly now. Previously this was happening internally. This is a pattern used by other glib-using projects, but if it's bad then it may be that something goes wrong with the file monitor.

[ Other information ]

I'll stage this following https://wiki.ubuntu.com/StableReleaseUpdates#Staging_an_upload.

Although this is a bug that could arguably affect users too (the fix is not limited to test code). So it may fix a real hang bug which we just didn't notice - or get reported - yet. I think it *could* be released to users.

[Sebastien Bacher (seb128)]

Olivier reported that one upstream but they didn't seem to have a clue about the issue

https://github.com/hughsie/libxmlb/issues/59

[Iain Lane (laney)]

It is uploaded

[Launchpad Janitor (janitor)]

This bug was fixed in the package libxmlb - 0.1.15-2ubuntu1

---------------
libxmlb (0.1.15-2ubuntu1) hirsute; urgency=medium

  * d/p/xb-silo-Cancel-the-GFileMonitor-before-unreffing-it.patch:
    Cherry-pick upstream patch to fix a deadlock. Forwarded to Debian at
    https://salsa.debian.org/efi-team/libxmlb/-/merge_requests (LP: #1890313)

 -- Iain Lane <email address hidden> Mon, 25 Jan 2021 16:01:47 +0000

[Robie Basak (racb)]

Hello Balint, or anyone else affected,

Accepted libxmlb into groovy-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/libxmlb/0.1.15-2ubuntu1~20.10.1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, what testing has been performed on the package and change the tag from verification-needed-groovy to verification-done-groovy. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-groovy. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

[Robie Basak (racb)]

Hello Balint, or anyone else affected,

Accepted libxmlb into focal-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/libxmlb/0.1.15-2ubuntu1~20.04.1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, what testing has been performed on the package and change the tag from verification-needed-focal to verification-done-focal. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-focal. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

[Robie Basak (racb)]

I renamed the bug to better describe the issue now that it has been root caused. Feel free to adjust if you disagree.

> Although this is a bug that could arguably affect users too (the fix is not limited to test code). So it may fix a real hang bug which we just didn't notice - or get reported - yet. I think it *could* be released to users.

I agree, and leave that to your discretion. Users affected seem unlikely to me to be in a position to track it back to this bug, so they won't be able to report themselves as affected. And the fix is trivial and limited in scope. So I think it's OK to release to users on the basis that it's much more likely to be useful to them when compared to the cost of the update to them and the risk of regressing them.

Feel free to untag block-proposed-{focal,groovy} if you think it's appropriate.

[Brian Murray (brian-murray)]

The fix for this bug has been awaiting testing feedback in the -proposed repository for focal for more than 90 days. Please test this fix and update the bug appropriately with the results. In the event that the fix for this bug is still not verified 15 days from now, the package will be removed from the -proposed repository.

[Iain Lane (laney)]

Ah, thanks for the reminder.

I've checked the autopkgtest runs for 0.1.15-2ubuntu1~20.04.1 and 0.1.15-2ubuntu1 and didn't find any failures. It's as good as we can get, I think, as outlined in the description.

I'll do the tags.

[Iain Lane (laney)]

The second version should be 0.1.15-2ubuntu1~20.10.1 (but 0.1.15-2ubuntu1 is in hirsute too with a good track record)

[Launchpad Janitor (janitor)]

This bug was fixed in the package libxmlb - 0.1.15-2ubuntu1~20.10.1

---------------
libxmlb (0.1.15-2ubuntu1~20.10.1) groovy; urgency=medium

  * No-change backport from hirsute to groovy

libxmlb (0.1.15-2ubuntu1) hirsute; urgency=medium

  * d/p/xb-silo-Cancel-the-GFileMonitor-before-unreffing-it.patch:
    Cherry-pick upstream patch to fix a deadlock. Forwarded to Debian at
    https://salsa.debian.org/efi-team/libxmlb/-/merge_requests (LP: #1890313)

 -- Iain Lane <email address hidden> Mon, 25 Jan 2021 16:05:07 +0000

[Łukasz Zemczak (sil2100)]

The verification of the Stable Release Update for libxmlb has completed successfully and the package is now being released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.

[Launchpad Janitor (janitor)]

This bug was fixed in the package libxmlb - 0.1.15-2ubuntu1~20.04.1

---------------
libxmlb (0.1.15-2ubuntu1~20.04.1) focal; urgency=medium

  * No-change backport from hirsute to focal

libxmlb (0.1.15-2ubuntu1) hirsute; urgency=medium

  * d/p/xb-silo-Cancel-the-GFileMonitor-before-unreffing-it.patch:
    Cherry-pick upstream patch to fix a deadlock. Forwarded to Debian at
    https://salsa.debian.org/efi-team/libxmlb/-/merge_requests (LP: #1890313)

 -- Iain Lane <email address hidden> Mon, 25 Jan 2021 16:05:07 +0000