[glib] gio: Don't leak the temp file when g_file_replace() fails or is cancelled



commit afdb2abb13896a3d5caecabd2f7158e8047f9956
Author: Dan Winship <danw gnome org>
Date:   Wed Nov 7 10:36:05 2012 -0500

    gio: Don't leak the temp file when g_file_replace() fails or is cancelled
    
    If the temp file still exists at the end of the close operation,
    unlink it.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=629301

 gio/glocalfileoutputstream.c |    5 ++
 gio/tests/file.c             |  136 +++++++++++++++++++++++++++++++++---------
 2 files changed, 113 insertions(+), 28 deletions(-)
---
diff --git a/gio/glocalfileoutputstream.c b/gio/glocalfileoutputstream.c
index a310fcd..59d8639 100644
--- a/gio/glocalfileoutputstream.c
+++ b/gio/glocalfileoutputstream.c
@@ -329,6 +329,8 @@ _g_local_file_output_stream_really_close (GLocalFileOutputStream *file,
 		       g_strerror (errsv));
 	  goto err_out;
 	}
+
+      g_clear_pointer (&file->priv->tmp_filename, g_free);
     }
   
   if (g_cancellable_set_error_if_cancelled (cancellable, error))
@@ -368,6 +370,9 @@ _g_local_file_output_stream_really_close (GLocalFileOutputStream *file,
   /* A simple try to close the fd in case we fail before the actual close */
   close (file->priv->fd);
 #endif
+  if (file->priv->tmp_filename)
+    g_unlink (file->priv->tmp_filename);
+
   return FALSE;
 }
 
diff --git a/gio/tests/file.c b/gio/tests/file.c
index e2d17d4..7106d89 100644
--- a/gio/tests/file.c
+++ b/gio/tests/file.c
@@ -466,6 +466,35 @@ test_create_delete (gconstpointer d)
   g_free (data);
 }
 
+static const gchar *replace_data =
+    "/**\n"
+    " * g_file_replace_contents_async:\n"
+    " * @file: input #GFile.\n"
+    " * @contents: string of contents to replace the file with.\n"
+    " * @length: the length of @contents in bytes.\n"
+    " * @etag: (allow-none): a new <link linkend=\"gfile-etag\">entity tag</link> for the @file, or %NULL\n"
+    " * @make_backup: %TRUE if a backup should be created.\n"
+    " * @flags: a set of #GFileCreateFlags.\n"
+    " * @cancellable: optional #GCancellable object, %NULL to ignore.\n"
+    " * @callback: a #GAsyncReadyCallback to call when the request is satisfied\n"
+    " * @user_data: the data to pass to callback function\n"
+    " * \n"
+    " * Starts an asynchronous replacement of @file with the given \n"
+    " * @contents of @length bytes. @etag will replace the document's\n"
+    " * current entity tag.\n"
+    " * \n"
+    " * When this operation has completed, @callback will be called with\n"
+    " * @user_user data, and the operation can be finalized with \n"
+    " * g_file_replace_contents_finish().\n"
+    " * \n"
+    " * If @cancellable is not %NULL, then the operation can be cancelled by\n"
+    " * triggering the cancellable object from another thread. If the operation\n"
+    " * was cancelled, the error %G_IO_ERROR_CANCELLED will be returned. \n"
+    " * \n"
+    " * If @make_backup is %TRUE, this function will attempt to \n"
+    " * make a backup of @file.\n"
+    " **/\n";
+
 typedef struct
 {
   GFile *file;
@@ -549,34 +578,7 @@ test_replace_load (void)
 
   data = g_new0 (ReplaceLoadData, 1);
   data->again = TRUE;
-  data->data =
-    "/**\n"
-    " * g_file_replace_contents_async:\n"
-    " * @file: input #GFile.\n"
-    " * @contents: string of contents to replace the file with.\n"
-    " * @length: the length of @contents in bytes.\n"
-    " * @etag: (allow-none): a new <link linkend=\"gfile-etag\">entity tag</link> for the @file, or %NULL\n"
-    " * @make_backup: %TRUE if a backup should be created.\n"
-    " * @flags: a set of #GFileCreateFlags.\n"
-    " * @cancellable: optional #GCancellable object, %NULL to ignore.\n"
-    " * @callback: a #GAsyncReadyCallback to call when the request is satisfied\n"
-    " * @user_data: the data to pass to callback function\n"
-    " * \n"
-    " * Starts an asynchronous replacement of @file with the given \n"
-    " * @contents of @length bytes. @etag will replace the document's\n"
-    " * current entity tag.\n"
-    " * \n"
-    " * When this operation has completed, @callback will be called with\n"
-    " * @user_user data, and the operation can be finalized with \n"
-    " * g_file_replace_contents_finish().\n"
-    " * \n"
-    " * If @cancellable is not %NULL, then the operation can be cancelled by\n"
-    " * triggering the cancellable object from another thread. If the operation\n"
-    " * was cancelled, the error %G_IO_ERROR_CANCELLED will be returned. \n"
-    " * \n"
-    " * If @make_backup is %TRUE, this function will attempt to \n"
-    " * make a backup of @file.\n"
-    " **/\n";
+  data->data = replace_data;
 
   data->file = g_file_new_tmp ("g_file_replace_load_XXXXXX",
 			       &iostream, NULL);
@@ -609,6 +611,81 @@ test_replace_load (void)
 }
 
 static void
+test_replace_cancel (void)
+{
+  GFile *tmpdir, *file;
+  GFileOutputStream *ostream;
+  GFileEnumerator *fenum;
+  GFileInfo *info;
+  GCancellable *cancellable;
+  gchar *path;
+  gsize nwrote;
+  GError *error = NULL;
+
+  g_test_bug ("629301");
+
+  path = g_dir_make_tmp ("g_file_replace_cancel_XXXXXX", &error);
+  g_assert_no_error (error);
+  tmpdir = g_file_new_for_path (path);
+  g_free (path);
+
+  file = g_file_get_child (tmpdir, "file");
+  g_file_replace_contents (file,
+                           replace_data,
+                           strlen (replace_data),
+                           NULL, FALSE, 0, NULL,
+                           NULL, &error);
+  g_assert_no_error (error);
+
+  ostream = g_file_replace (file, NULL, TRUE, 0, NULL, &error);
+  g_assert_no_error (error);
+
+  g_output_stream_write_all (G_OUTPUT_STREAM (ostream),
+                             replace_data, strlen (replace_data),
+                             &nwrote, NULL, &error);
+  g_assert_no_error (error);
+  g_assert_cmpint (nwrote, ==, strlen (replace_data));
+
+  /* At this point there should be two files; the original and the
+   * temporary.
+   */
+  fenum = g_file_enumerate_children (tmpdir, NULL, 0, NULL, &error);
+  g_assert_no_error (error);
+
+  info = g_file_enumerator_next_file (fenum, NULL, &error);
+  g_assert_no_error (error);
+  g_assert (info != NULL);
+  g_object_unref (info);
+  info = g_file_enumerator_next_file (fenum, NULL, &error);
+  g_assert_no_error (error);
+  g_assert (info != NULL);
+  g_object_unref (info);
+
+  g_file_enumerator_close (fenum, NULL, &error);
+  g_assert_no_error (error);
+  g_object_unref (fenum);
+
+  /* Make sure the temporary gets deleted even if we cancel. */
+  cancellable = g_cancellable_new ();
+  g_cancellable_cancel (cancellable);
+  g_output_stream_close (G_OUTPUT_STREAM (ostream), cancellable, &error);
+  g_assert_error (error, G_IO_ERROR, G_IO_ERROR_CANCELLED);
+  g_clear_error (&error);
+
+  g_object_unref (cancellable);
+  g_object_unref (ostream);
+
+  g_file_delete (file, NULL, &error);
+  g_assert_no_error (error);
+  g_object_unref (file);
+
+  /* This will only succeed if the temp file was deleted. */
+  g_file_delete (tmpdir, NULL, &error);
+  g_assert_no_error (error);
+  g_object_unref (tmpdir);
+}
+
+static void
 on_file_deleted (GObject      *object,
 		 GAsyncResult *result,
 		 gpointer      user_data)
@@ -655,6 +732,8 @@ main (int argc, char *argv[])
 {
   g_test_init (&argc, &argv, NULL);
 
+  g_test_bug_base ("http://bugzilla.gnome.org/";);
+
   g_test_add_func ("/file/basic", test_basic);
   g_test_add_func ("/file/parent", test_parent);
   g_test_add_func ("/file/child", test_child);
@@ -665,6 +744,7 @@ main (int argc, char *argv[])
   g_test_add_data_func ("/file/async-create-delete/25", GINT_TO_POINTER (25), test_create_delete);
   g_test_add_data_func ("/file/async-create-delete/4096", GINT_TO_POINTER (4096), test_create_delete);
   g_test_add_func ("/file/replace-load", test_replace_load);
+  g_test_add_func ("/file/replace-cancel", test_replace_cancel);
   g_test_add_func ("/file/async-delete", test_async_delete);
 
   return g_test_run ();



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]