GNOME.org
 

[gvfs] try extra hard to make sure operations get cancelled properly

commit c372064a04f8c24efdb99a3b0811bd17ddfb2176
Author: Benjamin Otte <otte gnome org>
Date:   Thu Jun 25 13:01:48 2009 +0200

    try extra hard to make sure operations get cancelled properly
    
    Previously there were some rather big windows that allowed for races
    between cancelling and calling g_simple_async_result_complete().
    This code makes sure we check for cancellaton right before calling
    g_simple_async_result_complete(), which gets rid of that window and
    gives you the guarantee that cancelling an operation in the main thread
    will indeed return a CANCELLED error.

 client/gdaemonfile.c             |   61 ++++++++++++++++++++++---------------
 client/gdaemonfileinputstream.c  |   12 +++++--
 client/gdaemonfileoutputstream.c |   10 ++++--
 client/gvfsdaemondbus.c          |   25 +++++++++++++++
 client/gvfsdaemondbus.h          |    4 ++
 client/gvfsiconloadable.c        |   14 ++++++--
 6 files changed, 90 insertions(+), 36 deletions(-)
---
diff --git a/client/gdaemonfile.c b/client/gdaemonfile.c
index 2da81f4..9c70b3c 100644
--- a/client/gdaemonfile.c
+++ b/client/gdaemonfile.c
@@ -580,7 +580,7 @@ async_path_call_done (DBusMessage *reply,
   if (io_error != NULL)
     {
       g_simple_async_result_set_from_error (data->result, io_error);
-      g_simple_async_result_complete (data->result);
+      _g_simple_async_result_complete_with_cancellable (data->result, data->cancellable);
       async_path_call_free (data);
     }
   else
@@ -613,7 +613,7 @@ do_async_path_call_callback (GMountInfo *mount_info,
   if (error != NULL)
     {
       g_simple_async_result_set_from_error (data->result, error);      
-      g_simple_async_result_complete (data->result);
+      _g_simple_async_result_complete_with_cancellable (data->result, data->cancellable);
       async_path_call_free (data);
       return;
     }
@@ -814,7 +814,7 @@ query_info_async_cb (DBusMessage *reply,
       g_simple_async_result_set_error (result,
 				       G_IO_ERROR, G_IO_ERROR_FAILED,
 				       _("Invalid return value from query_info"));
-      g_simple_async_result_complete (result);
+      _g_simple_async_result_complete_with_cancellable (result, cancellable);
       return;
     }
 
@@ -824,12 +824,12 @@ query_info_async_cb (DBusMessage *reply,
     {
       g_simple_async_result_set_from_error (result, error);
       g_error_free (error);
-      g_simple_async_result_complete (result);
+      _g_simple_async_result_complete_with_cancellable (result, cancellable);
       return;
     }
 
   g_simple_async_result_set_op_res_gpointer (result, info, g_object_unref);
-  g_simple_async_result_complete (result);
+  _g_simple_async_result_complete_with_cancellable (result, cancellable);
 }
 
 static void
@@ -877,6 +877,7 @@ g_daemon_file_query_info_finish (GFile                      *file,
 
 typedef struct {
   GSimpleAsyncResult *result;
+  GCancellable *cancellable;
   gboolean can_seek;
 } GetFDData;
 
@@ -899,7 +900,7 @@ read_async_get_fd_cb (int fd,
       g_simple_async_result_set_op_res_gpointer (data->result, stream, g_object_unref);
     }
 
-  g_simple_async_result_complete (data->result);
+  _g_simple_async_result_complete_with_cancellable (data->result, data->cancellable);
 
   g_object_unref (data->result);
   g_free (data);
@@ -924,7 +925,7 @@ read_async_cb (DBusMessage *reply,
       g_simple_async_result_set_error (result,
 				       G_IO_ERROR, G_IO_ERROR_FAILED,
 				       _("Invalid return value from open"));
-      g_simple_async_result_complete (result);
+      _g_simple_async_result_complete_with_cancellable (result, cancellable);
       return;
     }
   
@@ -1236,7 +1237,7 @@ mount_mountable_async_cb (DBusMessage *reply,
       g_simple_async_result_set_error (result,
 				       G_IO_ERROR, G_IO_ERROR_FAILED,
 				       _("Invalid return value from call"));
-      g_simple_async_result_complete (result);
+      _g_simple_async_result_complete_with_cancellable (result, cancellable);
 
       return;
     }
@@ -1253,7 +1254,7 @@ mount_mountable_async_cb (DBusMessage *reply,
 	  g_simple_async_result_set_error (result,
 					   G_IO_ERROR, G_IO_ERROR_FAILED,
 					   _("Invalid return value from call"));
-	  g_simple_async_result_complete (result);
+          _g_simple_async_result_complete_with_cancellable (result, cancellable);
 	  return;
 	}
       
@@ -1275,7 +1276,7 @@ mount_mountable_async_cb (DBusMessage *reply,
       
     }
   else
-    g_simple_async_result_complete (result);
+    _g_simple_async_result_complete_with_cancellable (result, cancellable);
 }
 
 static void
@@ -1332,7 +1333,7 @@ start_mountable_async_cb (DBusMessage *reply,
 			  GCancellable *cancellable,
 			  gpointer callback_data)
 {
-  g_simple_async_result_complete (result);
+  _g_simple_async_result_complete_with_cancellable (result, cancellable);
 }
 
 static void
@@ -1382,7 +1383,7 @@ stop_mountable_async_cb (DBusMessage *reply,
                          GCancellable *cancellable,
                          gpointer callback_data)
 {
-  g_simple_async_result_complete (result);
+  _g_simple_async_result_complete_with_cancellable (result, cancellable);
 }
 
 static void
@@ -1421,7 +1422,7 @@ eject_mountable_async_cb (DBusMessage *reply,
 			  GCancellable *cancellable,
 			  gpointer callback_data)
 {
-  g_simple_async_result_complete (result);
+  _g_simple_async_result_complete_with_cancellable (result, cancellable);
 }
 
 static void
@@ -1459,7 +1460,7 @@ unmount_mountable_async_cb (DBusMessage *reply,
 			    GCancellable *cancellable,
 			    gpointer callback_data)
 {
-  g_simple_async_result_complete (result);
+  _g_simple_async_result_complete_with_cancellable (result, cancellable);
 }
 
 static void
@@ -1495,6 +1496,7 @@ typedef struct {
   GFile *file;
   GMountOperation *mount_operation;
   GAsyncReadyCallback callback;
+  GCancellable *cancellable;
   gpointer user_data;
 } MountData;
 
@@ -1528,9 +1530,11 @@ mount_reply (DBusMessage *reply,
 				       g_daemon_file_mount_enclosing_volume);
     }
 
-  g_simple_async_result_complete (res);
+  _g_simple_async_result_complete_with_cancellable (res, data->cancellable);
   
   g_object_unref (data->file);
+  if (data->cancellable)
+    g_object_unref (data->cancellable);
   if (data->mount_operation)
     g_object_unref (data->mount_operation);
   g_free (data);
@@ -1570,6 +1574,8 @@ g_daemon_file_mount_enclosing_volume (GFile *location,
 
   data = g_new0 (MountData, 1);
   data->callback = callback;
+  if (data->cancellable)
+    data->cancellable = g_object_ref (data->cancellable);
   data->user_data = user_data;
   data->file = g_object_ref (location);
   if (mount_operation)
@@ -1650,7 +1656,7 @@ query_fs_info_async_cb (DBusMessage *reply,
       g_simple_async_result_set_error (result,
 				       G_IO_ERROR, G_IO_ERROR_FAILED,
 				       _("Invalid return value from query_info"));
-      g_simple_async_result_complete (result);
+      _g_simple_async_result_complete_with_cancellable (result, cancellable);
       return;
     }
 
@@ -1660,12 +1666,12 @@ query_fs_info_async_cb (DBusMessage *reply,
     {
       g_simple_async_result_set_from_error (result, error);
       g_error_free (error);
-      g_simple_async_result_complete (result);
+      _g_simple_async_result_complete_with_cancellable (result, cancellable);
       return;
     }
 
   g_simple_async_result_set_op_res_gpointer (result, info, g_object_unref);
-  g_simple_async_result_complete (result);
+  _g_simple_async_result_complete_with_cancellable (result, cancellable);
 }
 
 static void
@@ -2297,6 +2303,7 @@ g_daemon_file_monitor_file (GFile* file,
 typedef struct
 {
   GSimpleAsyncResult *result;
+  GCancellable       *cancellable;
   dbus_bool_t         can_seek;
   guint64             initial_offset;
 }
@@ -2318,7 +2325,9 @@ stream_open_cb (gint fd, StreamOpenParams *params)
   g_simple_async_result_set_op_res_gpointer (params->result, output_stream, g_object_unref);
 
 out:
-  g_simple_async_result_complete (params->result);
+  _g_simple_async_result_complete_with_cancellable (params->result, params->cancellable);
+  if (params->cancellable)
+    g_object_unref (params->cancellable);
   g_object_unref (params->result);
   g_slice_free (StreamOpenParams, params);
 }
@@ -2347,13 +2356,15 @@ append_to_async_cb (DBusMessage *reply,
     }
 
   open_params->result = g_object_ref (result);
+  if (cancellable)
+    open_params->cancellable = g_object_ref (cancellable);
   _g_dbus_connection_get_fd_async (connection, fd_id,
                                    (GetFdAsyncCallback) stream_open_cb, open_params);
   return;
 
 failure:
   g_slice_free (StreamOpenParams, open_params);
-  g_simple_async_result_complete (result);
+  _g_simple_async_result_complete_with_cancellable (result, cancellable);
 }
 
 static void
@@ -2431,7 +2442,7 @@ create_async_cb (DBusMessage *reply,
 
 failure:
   g_slice_free (StreamOpenParams, open_params);
-  g_simple_async_result_complete (result);
+  _g_simple_async_result_complete_with_cancellable (result, cancellable);
 }
 
 static void
@@ -2500,7 +2511,7 @@ enumerate_children_async_cb (DBusMessage *reply,
   g_simple_async_result_set_op_res_gpointer (result, enumerator, g_object_unref);
 
 out:
-  g_simple_async_result_complete (result);
+  _g_simple_async_result_complete_with_cancellable (result, cancellable);
 }
 
 static void
@@ -2608,7 +2619,7 @@ find_enclosing_mount_cb (GMountInfo *mount_info,
     }
 
 out:
-  g_simple_async_result_complete (data->result);
+  _g_simple_async_result_complete_with_cancellable (data->result, data->cancellable);
 
   if (my_error)
     g_error_free (my_error);
@@ -2690,7 +2701,7 @@ replace_async_cb (DBusMessage *reply,
 
 failure:
   g_slice_free (StreamOpenParams, open_params);
-  g_simple_async_result_complete (result);
+  _g_simple_async_result_complete_with_cancellable (result, cancellable);
 }
 
 static void
@@ -2765,7 +2776,7 @@ set_display_name_async_cb (DBusMessage *reply,
   g_simple_async_result_set_op_res_gpointer (result, file, g_object_unref);
 
 out:
-  g_simple_async_result_complete (result);
+  _g_simple_async_result_complete_with_cancellable (result, cancellable);
 }
 
 static void
diff --git a/client/gdaemonfileinputstream.c b/client/gdaemonfileinputstream.c
index 69f70c6..c942c9e 100644
--- a/client/gdaemonfileinputstream.c
+++ b/client/gdaemonfileinputstream.c
@@ -1631,6 +1631,7 @@ typedef void (*AsyncIteratorDone) (GInputStream *stream,
 				   gpointer op_data,
 				   GAsyncReadyCallback callback,
 				   gpointer callback_data,
+                                   GCancellable *cancellable,
 				   GError *io_error);
 
 struct AsyncIterator {
@@ -1654,6 +1655,7 @@ async_iterator_done (AsyncIterator *iterator, GError *io_error)
 		     iterator->iterator_data,
 		     iterator->callback,
 		     iterator->callback_data,
+                     iterator->cancellable,
 		     io_error);
 
   g_free (iterator);
@@ -1829,6 +1831,7 @@ async_read_done (GInputStream *stream,
 		 gpointer op_data,
 		 GAsyncReadyCallback callback,
 		 gpointer user_data,
+                 GCancellable *cancellable,
 		 GError *io_error)
 {
   ReadOperation *op;
@@ -1859,7 +1862,7 @@ async_read_done (GInputStream *stream,
     g_simple_async_result_set_from_error (simple, error);
 
   /* Complete immediately, not in idle, since we're already in a mainloop callout */
-  g_simple_async_result_complete (simple);
+  _g_simple_async_result_complete_with_cancellable (simple, cancellable);
   g_object_unref (simple);
   
   if (op->ret_error)
@@ -1941,6 +1944,7 @@ async_close_done (GInputStream *stream,
 		  gpointer op_data,
 		  GAsyncReadyCallback callback,
 		  gpointer user_data,
+                  GCancellable *cancellable,
 		  GError *io_error)
 {
   GDaemonFileInputStream *file;
@@ -1948,7 +1952,6 @@ async_close_done (GInputStream *stream,
   CloseOperation *op;
   gboolean result;
   GError *error;
-  GCancellable *cancellable = NULL; /* TODO: get cancellable */
 
   file = G_DAEMON_FILE_INPUT_STREAM (stream);
   
@@ -1984,7 +1987,7 @@ async_close_done (GInputStream *stream,
     g_simple_async_result_set_from_error (simple, error);
 
   /* Complete immediately, not in idle, since we're already in a mainloop callout */
-  g_simple_async_result_complete (simple);
+  _g_simple_async_result_complete_with_cancellable (simple, cancellable);
   g_object_unref (simple);
   
   if (op->ret_error)
@@ -2029,6 +2032,7 @@ async_query_done (GInputStream *stream,
 		  gpointer op_data,
 		  GAsyncReadyCallback callback,
 		  gpointer user_data,
+                  GCancellable *cancellable,
 		  GError *io_error)
 {
   GDaemonFileInputStream *file;
@@ -2063,7 +2067,7 @@ async_query_done (GInputStream *stream,
 					       g_object_unref);
   
   /* Complete immediately, not in idle, since we're already in a mainloop callout */
-  g_simple_async_result_complete (simple);
+  _g_simple_async_result_complete_with_cancellable (simple, cancellable);
   g_object_unref (simple);
   
   if (op->ret_error)
diff --git a/client/gdaemonfileoutputstream.c b/client/gdaemonfileoutputstream.c
index 27992bf..e47c2e8 100644
--- a/client/gdaemonfileoutputstream.c
+++ b/client/gdaemonfileoutputstream.c
@@ -1183,6 +1183,7 @@ typedef void (*AsyncIteratorDone) (GOutputStream *stream,
 				   gpointer op_data,
 				   GAsyncReadyCallback callback,
 				   gpointer callback_data,
+                                   GCancellable *cancellable,
 				   GError *io_error);
 
 struct AsyncIterator {
@@ -1206,6 +1207,7 @@ async_iterator_done (AsyncIterator *iterator, GError *io_error)
 		     iterator->iterator_data,
 		     iterator->callback,
 		     iterator->callback_data,
+                     iterator->cancellable,
 		     io_error);
 
   g_free (iterator);
@@ -1381,6 +1383,7 @@ async_write_done (GOutputStream *stream,
 		  gpointer op_data,
 		  GAsyncReadyCallback callback,
 		  gpointer user_data,
+                  GCancellable *cancellable,
 		  GError *io_error)
 {
   GSimpleAsyncResult *simple;
@@ -1411,7 +1414,7 @@ async_write_done (GOutputStream *stream,
     g_simple_async_result_set_from_error (simple, error);
 
   /* Complete immediately, not in idle, since we're already in a mainloop callout */
-  g_simple_async_result_complete (simple);
+  _g_simple_async_result_complete_with_cancellable (simple, cancellable);
   g_object_unref (simple);
 
   if (op->ret_error)
@@ -1513,7 +1516,7 @@ async_close_done (GOutputStream *stream,
     g_simple_async_result_set_from_error (simple, error);
 
   /* Complete immediately, not in idle, since we're already in a mainloop callout */
-  g_simple_async_result_complete (simple);
+  _g_simple_async_result_complete_with_cancellable (simple, cancellable);
   g_object_unref (simple);
   
   if (op->ret_error)
@@ -1558,6 +1561,7 @@ async_query_done (GOutputStream *stream,
 		  gpointer op_data,
 		  GAsyncReadyCallback callback,
 		  gpointer user_data,
+                  GCancellable *cancellable,
 		  GError *io_error)
 {
   GDaemonFileOutputStream *file;
@@ -1592,7 +1596,7 @@ async_query_done (GOutputStream *stream,
 					       g_object_unref);
   
   /* Complete immediately, not in idle, since we're already in a mainloop callout */
-  g_simple_async_result_complete (simple);
+  _g_simple_async_result_complete_with_cancellable (simple, cancellable);
   g_object_unref (simple);
   
   if (op->ret_error)
diff --git a/client/gvfsdaemondbus.c b/client/gvfsdaemondbus.c
index a9dae13..4675d4f 100644
--- a/client/gvfsdaemondbus.c
+++ b/client/gvfsdaemondbus.c
@@ -1024,3 +1024,28 @@ _g_dbus_connection_get_sync (const char *dbus_id,
 
   return connection;
 }
+
+/**
+ * _g_simple_async_result_complete_with_cancellable:
+ * @result: the result
+ * @cancellable: a cancellable to check
+ *
+ * If @cancellable is cancelled, sets @result into the cancelled error
+ * state. Then calls g_simple_async_result_complete().
+ * This function is useful to ensure that @result is properly set into
+ * an error state on cancellation.
+ **/
+void
+_g_simple_async_result_complete_with_cancellable (GSimpleAsyncResult *result,
+                                                  GCancellable       *cancellable)
+{
+  if (cancellable &&
+      g_cancellable_is_cancelled (cancellable))
+    g_simple_async_result_set_error (result,
+                                     G_IO_ERROR,
+                                     G_IO_ERROR_CANCELLED,
+                                     "%s", _("Operation was cancelled"));
+
+  g_simple_async_result_complete (result);
+}
+
diff --git a/client/gvfsdaemondbus.h b/client/gvfsdaemondbus.h
index 7a132cd..c60108f 100644
--- a/client/gvfsdaemondbus.h
+++ b/client/gvfsdaemondbus.h
@@ -75,6 +75,10 @@ DBusMessage *   _g_vfs_daemon_call_sync                 (DBusMessage
 GFileInfo *     _g_dbus_get_file_info                   (DBusMessageIter                *iter,
 							 GError                        **error);
 
+void        _g_simple_async_result_complete_with_cancellable
+                                                        (GSimpleAsyncResult             *result,
+                                                         GCancellable                   *cancellable);
+
 G_END_DECLS
 
 #endif /* __G_VFS_DAEMON_DBUS_H__ */
diff --git a/client/gvfsiconloadable.c b/client/gvfsiconloadable.c
index 1c6d54b..28251cf 100644
--- a/client/gvfsiconloadable.c
+++ b/client/gvfsiconloadable.c
@@ -208,7 +208,7 @@ async_path_call_done (DBusMessage *reply,
   if (io_error != NULL)
     {
       g_simple_async_result_set_from_error (data->result, io_error);
-      g_simple_async_result_complete (data->result);
+      _g_simple_async_result_complete_with_cancellable (data->result, data->cancellable);
       async_path_call_free (data);
     }
   else
@@ -239,7 +239,7 @@ do_async_path_call_callback (GMountInfo *mount_info,
   if (error != NULL)
     {
       g_simple_async_result_set_from_error (data->result, error);      
-      g_simple_async_result_complete (data->result);
+      _g_simple_async_result_complete_with_cancellable (data->result, data->cancellable);
       async_path_call_free (data);
       return;
     }
@@ -321,6 +321,7 @@ do_async_path_call (GVfsIcon *vfs_icon,
 
 typedef struct {
   GSimpleAsyncResult *result;
+  GCancellable *cancellable;
   gboolean can_seek;
 } GetFDData;
 
@@ -343,9 +344,12 @@ load_async_get_fd_cb (int fd,
       g_simple_async_result_set_op_res_gpointer (data->result, stream, g_object_unref);
     }
 
-  g_simple_async_result_complete (data->result);
+  _g_simple_async_result_complete_with_cancellable (data->result,
+                                                    data->cancellable);
 
   g_object_unref (data->result);
+  if (data->cancellable)
+    g_object_unref (data->cancellable);
   g_free (data);
 }
 
@@ -368,12 +372,14 @@ load_async_cb (DBusMessage *reply,
       g_simple_async_result_set_error (result,
 				       G_IO_ERROR, G_IO_ERROR_FAILED,
 				       _("Invalid return value from open"));
-      g_simple_async_result_complete (result);
+      _g_simple_async_result_complete_with_cancellable (result, cancellable);
       return;
     }
 
   get_fd_data = g_new0 (GetFDData, 1);
   get_fd_data->result = g_object_ref (result);
+  if (cancellable)
+    get_fd_data->cancellable = g_object_ref (cancellable);
   get_fd_data->can_seek = can_seek;
 
   _g_dbus_connection_get_fd_async (connection, fd_id,
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]