Skip to content

GSettingsBackend watches not thread-safe

See https://github.com/gtk-rs/gio/pull/226 for a testcase. Running

cargo test -- settings::test

in a loop will at some point explode with

(process:7244): GLib-GObject-CRITICAL **: 16:50:33.710: g_object_ref: assertion 'G_IS_OBJECT (object)' failed


(process:7244): GLib-GObject-CRITICAL **: 16:50:33.710: g_object_unref: assertion 'G_IS_OBJECT (object)' failed
**
GLib-GObject:ERROR:../gobject/gobject.c:4453:g_weak_ref_set: assertion failed: (weak_locations != NULL)
error: process didn't exit successfully: `/home/rasmus/Projects/gio/target/debug/deps/gio-8e55b7fc5fe6fb99` (signal: 6, SIGABRT: process abort signal)

Backtrace in question for the first critical is

Thread 1 (Thread 0x7f57fecf5700 (LWP 31753)):
#0  0x00007f57ff504885 in _g_log_abort (breakpoint=1) at ../../../glib/gmessages.c:554
#1  0x00007f57ff505b8d in g_logv (log_domain=0x7f57ff613258 "GLib-GObject", log_level=G_LOG_LEVEL_CRITICAL, format=<optimized out>, args=args@entry=0x7f57fecf41a0) at ../../../glib/gmessages.c:1371
#2  0x00007f57ff505d5f in g_log (log_domain=log_domain@entry=0x7f57ff613258 "GLib-GObject", log_level=log_level@entry=G_LOG_LEVEL_CRITICAL, format=format@entry=0x7f57ff55557c "%s: assertion '%s' failed") at ../../../glib/gmessages.c:1413
#3  0x00007f57ff506559 in g_return_if_fail_warning (log_domain=log_domain@entry=0x7f57ff613258 "GLib-GObject", pretty_function=pretty_function@entry=0x7f57ff6167f8 <__FUNCTION__.15104> "g_object_ref", expression=expression@entry=0x7f57ff61559f "G_IS_OBJECT (object)") at ../../../glib/gmessages.c:2767
#4  0x00007f57ff5e9c4c in g_object_ref (_object=0x7f57e8002170) at ../../../gobject/gobject.c:3212
#5  0x00007f57ff5efbc5 in g_weak_ref_get (weak_ref=<optimized out>) at ../../../gobject/gobject.c:4393
#6  0x00007f57ff712df2 in g_settings_backend_invoke_closure (user_data=0x7f57e8009e80) at ../../../gio/gsettingsbackend.c:263
#7  0x00007f57ff4ffa05 in g_main_context_invoke_full (context=0x7f57f80061e0, priority=0, function=0x7f57ff712de0 <g_settings_backend_invoke_closure>, data=0x7f57e8009e80, notify=0x0) at ../../../glib/gmain.c:5843
#8  0x00007f57ff4ffa50 in g_main_context_invoke (context=<optimized out>, function=<optimized out>, data=<optimized out>) at ../../../glib/gmain.c:5788
#9  0x00007f57ff712f68 in g_settings_backend_dispatch_signal (backend=0x7f57e8009510 [GMemorySettingsBackend], function_offset=0, name=0x55cc920c0760 "/com/github/gtk-rs/test-bool", origin_tag=0x0, names=0x0) at ../../../gio/gsettingsbackend.c:330
#10 0x00007f57ff7135bb in g_settings_backend_changed (backend=<optimized out>, key=<optimized out>, origin_tag=<optimized out>) at ../../../gio/gsettingsbackend.c:379
#11 0x00007f57ff7129ce in g_memory_settings_backend_write (backend=0x7f57e8009510 [GMemorySettingsBackend], key=0x55cc920c0760 "/com/github/gtk-rs/test-bool", value=0x7f57f8009060, origin_tag=0x0) at ../../../gio/gmemorysettingsbackend.c:80
#12 0x00007f57ff713b24 in g_settings_backend_write (backend=0x7f57e8009510 [GMemorySettingsBackend], key=key@entry=0x55cc920c0760 "/com/github/gtk-rs/test-bool", value=value@entry=0x7f57f8009060, origin_tag=origin_tag@entry=0x0) at ../../../gio/gsettingsbackend.c:793
#13 0x00007f57ff718023 in g_settings_write_to_backend (settings=settings@entry=0x7f57f8007430 [GSettings], value=value@entry=0x7f57f8009060, key=<optimized out>) at ../../../gio/gsettings.c:1151
#14 0x00007f57ff719d78 in g_settings_set_value (settings=0x7f57f8007430 [GSettings], key=0x7f57f80061a0 "test-bool", value=0x7f57f8009060) at ../../../gio/gsettings.c:1591

The reason why this happens is relatively clear:

  1. g_settings_backend_watch() uses a weak notify for keeping track of the target. There's an explanation why this is supposed to be safe but that explanation is wrong.
  2. We have the target stored in the watch list
  3. The last reference to the target is dropped in thread A and we end up in g_settings_backend_watch_weak_notify() right before the mutex
  4. g_settings_backend_dispatch_signal() is called from another thread B and gets the mutex before 3)
  5. g_weak_ref_init() is called on the target from thread B, which at this point has a reference count of exactly one (see g_object_unref() where it calls the weak notifies)
  6. Thread A continues at 3) and drops the last reference and destroys the object. Now the GWeakRef from 5) points to a destroyed object. Note that GWeakRefs would be cleared before the weak notifies are called
  7. At some later point another thread g_weak_ref_get() is called by g_settings_backend_invoke_closure() and accesses an already destroyed object with refcount 0 from the GWeakRef created in 5) by thread B (or worse, already freed memory that was reused).

Unclear how to fix that properly though. Using a GWeakRef instead of the weak notify doesn't work because we need some kind of callback to clean up a list of things. Unless we set up some kind of "garbage collection" thread that goes through that list periodically and cleans up anything that has an empty GWeakRef.

Ideas?

Edited by Sebastian Dröge