Skip to content

GSettingsBackend - Fix thread-safety during destruction of GSettings instances...

Sebastian Dröge requested to merge sdroege/glib:gsettingsbackend-thread-safe-watches into master

GSettingsBackend - Fix thread-safety during destruction of GSettings instances while notifications are emitted

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.

The following could happen before:

  1. We have the target stored in the watch list
  2. 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
  3. g_settings_backend_dispatch_signal() is called from another thread B and gets the mutex before 2.
  4. 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)
  5. Thread A continues at 3. and drops the last reference and destroys the object. Now the GWeakRef from 4. points to a destroyed object. Note that GWeakRefs would be cleared before the weak notifies are called
  6. 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 4. by thread B (or worse, already freed memory that was reused).

Solve this by actually storing a GWeakRef of the target in the watch list and only access the target behind it via the GWeakRef API, and then pass a strong reference to the notification dispatch code.

The weak notify is only used to remove the (potentially with empty GWeakRef) target from the list of watches and the only place that compares the target by pointer instead of going through the GWeakRef API.

Fixes #1870 (closed)

Merge request reports