[glib/benzea/fix-inotify-race: 7/7] glocalfilemonitor: Avoid file monitor destruction from event thread

[Benjamin Berg <bberg src gnome org>]

commit 012acf12259d5a0934bd9e5d6ca4a367f2391d8c
Author: Benjamin Berg <bberg redhat com>
Date:   Fri Mar 18 12:16:12 2022 +0100

    glocalfilemonitor: Avoid file monitor destruction from event thread
    
    Taking a reference to the GFileMonitor when handling events may cause
    object destruction from th worker thread that calls the function. This
    condition happens if the surrounding code drops the otherwise last
    reference ot the GFileMonitor. The series of events causes destruction
    from an unrelated worker thread and also triggers g_file_monitor_cancel
    to be called from g_file_monitor_source_handle_event.
    
    For the inotify backend, this results in a deadlock as cancellation
    needs to take a lock that protects data structures from being modified
    while events are dispatched.
    
    One alternative to this approach might be to add an RCU (release, copy,
    update) approach to the lists contained in the wd_dir_hash and
    wd_file_hash hash tables.
    
    Fixes: #1941
    
    An example stack trace of this happening is:
    
    Thread 2 (Thread 0x7fea68b1d640 (LWP 260961) "gmain"):
     #0  syscall () at ../sysdeps/unix/sysv/linux/x86_64/syscall.S:38
     #1  0x00007fea692215dc in g_mutex_lock_slowpath (mutex=mutex@entry=0x7fea6911e148 <g.inotify_lock_lock>) 
at ../glib/gthread-posix.c:1493
     #2  0x00007fea69222062 in g_mutex_lock (mutex=mutex@entry=0x7fea6911e148 <g.inotify_lock_lock>) at 
../glib/gthread-posix.c:1517
     #3  0x00007fea6908025a in _ih_sub_cancel (sub=0x1492620) at ../gio/inotify/inotify-helper.c:131
     #4  0x00007fea6907f9da in g_inotify_file_monitor_cancel (monitor=0x14a3550) at 
../gio/inotify/ginotifyfilemonitor.c:75
     #5  0x00007fea68fae959 in g_file_monitor_cancel (monitor=0x14a3550) at ../gio/gfilemonitor.c:241
     #6  0x00007fea68fae9dc in g_file_monitor_dispose (object=0x14a3550) at ../gio/gfilemonitor.c:123
     #7  0x00007fea69139341 in g_object_unref (_object=<optimized out>) at ../gobject/gobject.c:3636
     #8  g_object_unref (_object=0x14a3550) at ../gobject/gobject.c:3553
     #9  0x00007fea6907507a in g_file_monitor_source_handle_event (fms=0x14c3560, event_type=<optimized out>, 
child=0x7fea64001460 "spawned-1", rename_to=rename_to@entry=0x0, other=other@entry=0x0, event_time=<optimized 
out>) at ../gio/glocalfilemonitor.c:457
     #10 0x00007fea6907fe0e in ih_event_callback (event=0x7fea64001420, sub=0x1492620, file_event=<optimized 
out>) at ../gio/inotify/inotify-helper.c:218
     #11 0x00007fea6908075c in ip_event_dispatch (dir_list=dir_list@entry=0x14c14c0, file_list=0x0, 
event=event@entry=0x7fea64001420) at ../gio/inotify/inotify-path.c:493
     #12 0x00007fea6908094e in ip_event_dispatch (event=0x7fea64001420, file_list=<optimized out>, 
dir_list=0x14c14c0) at ../gio/inotify/inotify-path.c:448
     #13 ip_event_callback (event=0x7fea64001420) at ../gio/inotify/inotify-path.c:548
     #14 ip_event_callback (event=0x7fea64001420) at ../gio/inotify/inotify-path.c:530
     #15 0x00007fea69081391 in ik_source_dispatch (source=0x14a2bf0, func=0x7fea69080890 <ip_event_callback>, 
user_data=<optimized out>) at ../gio/inotify/inotify-kernel.c:327
     #16 0x00007fea691d0824 in g_main_dispatch (context=0x14a2cc0) at ../glib/gmain.c:3417
     #17 g_main_context_dispatch (context=0x14a2cc0) at ../glib/gmain.c:4135
     #18 0x00007fea691d0b88 in g_main_context_iterate (context=context@entry=0x14a2cc0, block=block@entry=1, 
dispatch=dispatch@entry=1, self=<optimized out>) at ../glib/gmain.c:4211
     #19 0x00007fea691d0c2f in g_main_context_iteration (context=0x14a2cc0, may_block=may_block@entry=1) at 
../glib/gmain.c:4276
     #20 0x00007fea691d0c81 in glib_worker_main (data=<optimized out>) at ../glib/gmain.c:6176
     #21 0x00007fea691f9c2d in g_thread_proxy (data=0x1487cc0) at ../glib/gthread.c:827
     #22 0x00007fea68d93b1a in start_thread (arg=<optimized out>) at pthread_create.c:443
     #23 0x00007fea68e18650 in clone3 () at ../sysdeps/unix/sysv/linux/x86_64/clone3.S:81

 gio/glocalfilemonitor.c | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)
---
diff --git a/gio/glocalfilemonitor.c b/gio/glocalfilemonitor.c
index 4f85fea52..f408d0707 100644
--- a/gio/glocalfilemonitor.c
+++ b/gio/glocalfilemonitor.c
@@ -348,7 +348,6 @@ g_file_monitor_source_handle_event (GFileMonitorSource *fms,
                                     gint64              event_time)
 {
   gboolean interesting = TRUE;
-  GFileMonitor *instance = NULL;
 
   g_assert (!child || is_basename (child));
   g_assert (!rename_to || is_basename (rename_to));
@@ -359,13 +358,11 @@ g_file_monitor_source_handle_event (GFileMonitorSource *fms,
 
   g_mutex_lock (&fms->lock);
 
-  /* monitor is already gone -- don't bother */
-  instance = g_weak_ref_get (&fms->instance_ref);
-  if (instance == NULL)
-    {
-      g_mutex_unlock (&fms->lock);
-      return TRUE;
-    }
+  /* NOTE: We process events even if the file monitor has already been disposed.
+   *       The reason is that we must not take a reference to the instance here
+   *       as destroying it from the event handling thread will lead to a
+   *       deadlock when taking the lock in _ih_sub_cancel.
+   */
 
   switch (event_type)
     {
@@ -452,7 +449,6 @@ g_file_monitor_source_handle_event (GFileMonitorSource *fms,
   g_file_monitor_source_update_ready_time (fms);
 
   g_mutex_unlock (&fms->lock);
-  g_clear_object (&instance);
 
   return interesting;
 }