Bug 545489 – Nautilus hangs when inserting a CD

Fix callback with lock held and various ref counting
issues.

svn path=/trunk/; revision=1860

3 files changed, 57 insertions, 33 deletions

diff --git a/ChangeLog b/ChangeLog
index 09c30f51..fea1fb78 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,16 @@
+2008-08-05  Christian Kellner  <gicmo@gnome.org>
+
+	* monitor/hal/ghaldrive.c: 
+	Various ref counting fixes.
+
+	* monitor/hal/ghalvolume.c:
+	Report error in idle to avoid calling a callback with the
+	volume lock held.
+	Use volume lock in hhal_volume_get_activation_root. Various
+	ref counting fixes.
+
+	Bug #545489. Based on a patch from Matthias Clasen.
+
 2008-08-04  Christian Kellner  <gicmo@gnome.org>
 
 	* client/gdaemonfile.c: (g_daemon_file_replace_async):
diff --git a/monitor/hal/ghaldrive.c b/monitor/hal/ghaldrive.c
index 7a85e888..6b2629a7 100644
--- a/monitor/hal/ghaldrive.c
+++ b/monitor/hal/ghaldrive.c
@@ -1,5 +1,5 @@
 /* GIO - GLib Input, Output and Streaming Library
- * 
+ *
  * Copyright (C) 2006-2007 Red Hat, Inc.
  *
  * This library is free software; you can redistribute it and/or
@@ -623,6 +623,7 @@ spawn_cb (GPid pid, gint status, gpointer user_data)
     }
   g_simple_async_result_complete (simple);
   g_object_unref (simple);
+  g_object_unref (data->object);
   g_free (data);
 }
 
@@ -647,7 +648,7 @@ g_hal_drive_eject_do (GDrive              *drive,
   data->callback = callback;
   data->user_data = user_data;
   data->cancellable = cancellable;
-  
+
   error = NULL;
   if (!g_spawn_async (NULL,         /* working dir */
                       argv,
@@ -659,19 +660,20 @@ g_hal_drive_eject_do (GDrive              *drive,
                       &error))
     {
       GSimpleAsyncResult *simple;
-      
+
       simple = g_simple_async_result_new_from_error (data->object,
                                                      data->callback,
                                                      data->user_data,
                                                      error);
       g_simple_async_result_complete (simple);
       g_object_unref (simple);
+      g_object_unref (drive);
       g_error_free (error);
       g_free (data);
     }
   else
     g_child_watch_add (child_pid, spawn_cb, data);
-    
+
   g_free (argv[4]);
 }
 
@@ -717,10 +719,10 @@ _eject_unmount_mounts_cb (GObject *source_object,
       if (error->code != G_IO_ERROR_FAILED_HANDLED)
         {
           g_error_free (error);
-          error = g_error_new (G_IO_ERROR, G_IO_ERROR_BUSY, 
+          error = g_error_new (G_IO_ERROR, G_IO_ERROR_BUSY,
                                _("Failed to eject media; one or more volumes on the media are busy."));
         }
-      
+
       /* unmount failed; need to fail the whole eject operation */
       simple = g_simple_async_result_new_from_error (G_OBJECT (data->drive),
                                                      data->callback,
@@ -758,6 +760,8 @@ _eject_unmount_mounts (UnmountMountsOp *data)
                             data->cancellable,
                             data->callback,
                             data->user_data);
+
+      g_object_unref (data->drive);
       g_free (data);
     }
   else
@@ -789,7 +793,7 @@ g_hal_drive_eject (GDrive              *drive,
   /* first we need to go through all the volumes and unmount their assoicated mounts (if any) */
 
   data = g_new0 (UnmountMountsOp, 1);
-  data->drive = drive;
+  data->drive = g_object_ref (drive);
   data->cancellable = cancellable;
   data->callback = callback;
   data->user_data = user_data;
@@ -831,14 +835,14 @@ poll_for_media_cb (DBusPendingCall *pending_call, void *user_data)
   PollOp *data = (PollOp *) user_data;
   GSimpleAsyncResult *simple;
   DBusMessage *reply;
-  
+
   reply = dbus_pending_call_steal_reply (pending_call);
-  
+
   if (dbus_message_get_type (reply) == DBUS_MESSAGE_TYPE_ERROR)
     {
       GError *error;
       DBusError dbus_error;
-      
+
       dbus_error_init (&dbus_error);
       dbus_set_error_from_message (&dbus_error, reply);
       error = g_error_new (G_IO_ERROR,
@@ -855,18 +859,19 @@ poll_for_media_cb (DBusPendingCall *pending_call, void *user_data)
       goto out;
     }
 
-  /* TODO: parse reply and extract result? 
-   * (the result is whether the media availability state changed) 
+  /* TODO: parse reply and extract result?
+   * (the result is whether the media availability state changed)
    */
-  
+
   simple = g_simple_async_result_new (data->object,
                                       data->callback,
                                       data->user_data,
                                       NULL);
   g_simple_async_result_complete (simple);
   g_object_unref (simple);
-  
+
  out:
+  g_object_unref (data->object);
   dbus_message_unref (reply);
   dbus_pending_call_unref (pending_call);
 }
@@ -885,7 +890,7 @@ g_hal_drive_poll_for_media (GDrive              *drive,
   PollOp *data;
 
   data = g_new0 (PollOp, 1);
-  data->object = G_OBJECT (drive);
+  data->object = g_object_ref (drive);
   data->callback = callback;
   data->user_data = user_data;
   data->cancellable = cancellable;
@@ -894,12 +899,12 @@ g_hal_drive_poll_for_media (GDrive              *drive,
 
   G_LOCK (hal_drive);
   con = hal_pool_get_dbus_connection (hal_drive->pool);
-  msg = dbus_message_new_method_call ("org.freedesktop.Hal", 
+  msg = dbus_message_new_method_call ("org.freedesktop.Hal",
                                       hal_device_get_udi (hal_drive->device),
                                       "org.freedesktop.Hal.Device.Storage.Removable",
                                       "CheckForMedia");
   G_UNLOCK (hal_drive);
-  
+
   if (!dbus_connection_send_with_reply (con, msg, &pending_call, -1))
     {
       GError *error;
@@ -914,6 +919,7 @@ g_hal_drive_poll_for_media (GDrive              *drive,
       g_simple_async_result_complete (simple);
       g_object_unref (simple);
       g_error_free (error);
+      g_object_unref (data->object);
       g_free (data);
     }
   else
@@ -921,7 +927,7 @@ g_hal_drive_poll_for_media (GDrive              *drive,
                                   poll_for_media_cb,
                                   data,
                                   (DBusFreeFunction) g_free);
-  
+
   dbus_message_unref (msg);
 }
 
diff --git a/monitor/hal/ghalvolume.c b/monitor/hal/ghalvolume.c
index 078e6d04..4c9e43f5 100644
--- a/monitor/hal/ghalvolume.c
+++ b/monitor/hal/ghalvolume.c
@@ -1,5 +1,5 @@
 /* GIO - GLib Input, Output and Streaming Library
- * 
+ *
  * Copyright (C) 2006-2007 Red Hat, Inc.
  *
  * This library is free software; you can redistribute it and/or
@@ -798,14 +798,11 @@ spawn_do (GVolume             *volume,
                       &child_pid,
                       &error))
     {
-      GSimpleAsyncResult *simple;
-      simple = g_simple_async_result_new_from_error (data->object,
-                                                     data->callback,
-                                                     data->user_data,
-                                                     error);
+      g_simple_async_report_gerror_in_idle (data->object,
+                                            data->callback,
+                                            data->user_data,
+                                            error);
       g_object_unref (data->object);
-      g_simple_async_result_complete (simple);
-      g_object_unref (simple);
       g_error_free (error);
       g_free (data);
       return;
@@ -828,6 +825,7 @@ mount_foreign_callback (GObject *source_object,
 {
   ForeignMountOp *data = user_data;
   data->callback (G_OBJECT (data->enclosing_volume), res, data->user_data);
+  g_object_unref (data->enclosing_volume);
   g_free (data);
 }
 
@@ -852,7 +850,7 @@ g_hal_volume_mount (GVolume             *volume,
       ForeignMountOp *data;
 
       data = g_new0 (ForeignMountOp, 1);
-      data->enclosing_volume = hal_volume;
+      data->enclosing_volume = g_object_ref (hal_volume);
       data->callback = callback;
       data->user_data = user_data;
 
@@ -907,6 +905,7 @@ eject_wrapper_callback (GObject *source_object,
 {
   EjectWrapperOp *data  = user_data;
   data->callback (data->object, res, data->user_data);
+  g_object_unref (data->object);
   g_free (data);
 }
 
@@ -927,7 +926,7 @@ g_hal_volume_eject (GVolume              *volume,
   if (hal_volume->drive != NULL)
     drive = g_object_ref (hal_volume->drive);
   G_UNLOCK (hal_volume);
-  
+
   if (drive != NULL)
     {
       EjectWrapperOp *data;
@@ -986,7 +985,7 @@ g_hal_volume_enumerate_identifiers (GVolume *volume)
   const char *label, *uuid;
 
   G_LOCK (hal_volume);
-  
+
   res = g_ptr_array_new ();
 
   g_ptr_array_add (res,
@@ -995,10 +994,10 @@ g_hal_volume_enumerate_identifiers (GVolume *volume)
   if (hal_volume->device_path && *hal_volume->device_path != 0)
     g_ptr_array_add (res,
                      g_strdup (G_VOLUME_IDENTIFIER_KIND_UNIX_DEVICE));
-    
+
   label = hal_device_get_property_string (hal_volume->device, "volume.label");
   uuid = hal_device_get_property_string (hal_volume->device, "volume.uuid");
-  
+
   if (label && *label != 0)
     g_ptr_array_add (res,
                      g_strdup (G_VOLUME_IDENTIFIER_KIND_LABEL));
@@ -1011,7 +1010,7 @@ g_hal_volume_enumerate_identifiers (GVolume *volume)
   g_ptr_array_add (res, NULL);
 
   G_UNLOCK (hal_volume);
-  
+
   return (char **)g_ptr_array_free (res, FALSE);
 }
 
@@ -1019,8 +1018,14 @@ static GFile *
 g_hal_volume_get_activation_root (GVolume *volume)
 {
   GHalVolume *hal_volume = G_HAL_VOLUME (volume);
+  GFile *root = NULL;
+
+  G_LOCK (hal_volume);
+  if (hal_volume->foreign_mount_root != NULL)
+    root = g_object_ref (hal_volume->foreign_mount_root);
+  G_UNLOCK (hal_volume);
 
-  return hal_volume->foreign_mount_root != NULL ? g_object_ref (hal_volume->foreign_mount_root) : NULL;
+  return root;
 }
 
 static void