Bug 575555 – Use fsync() when replacing files to avoid data loss on

2009-03-16  Alexander Larsson  <alexl@redhat.com>

	Bug 575555 – Use fsync() when replacing files to avoid data loss on crash

        * configure.in:
	Look for fsync().

        * glib/gfileutils.c:
        (write_to_temp_file):
	fsync temp file if destination file exists

2009-03-16  Alexander Larsson  <alexl@redhat.com>

	Bug 575555 – Use fsync() when replacing files to avoid data loss on crash

        * glocalfileoutputstream.c:
        (g_local_file_output_stream_close):
        (_g_local_file_output_stream_replace):
	fsync temp file before closing if replacing target file

svn path=/trunk/; revision=7991

diff --git a/ChangeLog b/ChangeLog
index 9339cf9..a97312a 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,14 @@
+2009-03-16  Alexander Larsson  <alexl@redhat.com>
+
+       Bug 575555 – Use fsync() when replacing files to avoid data loss on crash
+
+        * configure.in:
+       Look for fsync().
+
+        * glib/gfileutils.c:
+        (write_to_temp_file):
+       fsync temp file if destination file exists
+
 2009-03-13  Matthias Clasen  <mclasen@redhat.com>
 
        * configure.in: Bump version

diff --git a/configure.in b/configure.in
index 186fd21..9922b50 100644 (file)
--- a/configure.in
+++ b/configure.in
@@ -563,6 +563,7 @@ AC_CHECK_FUNCS(mmap)
 AC_CHECK_FUNCS(posix_memalign)
 AC_CHECK_FUNCS(memalign)
 AC_CHECK_FUNCS(valloc)
+AC_CHECK_FUNCS(fsync)
 
 AC_CHECK_FUNCS(atexit on_exit)
 

diff --git a/gio/ChangeLog b/gio/ChangeLog
index 604e38d..8f73481 100644 (file)
--- a/gio/ChangeLog
+++ b/gio/ChangeLog
@@ -1,3 +1,12 @@
+2009-03-16  Alexander Larsson  <alexl@redhat.com>
+
+       Bug 575555 – Use fsync() when replacing files to avoid data loss on crash
+
+        * glocalfileoutputstream.c:
+        (g_local_file_output_stream_close):
+        (_g_local_file_output_stream_replace):
+       fsync temp file before closing if replacing target file
+
 2009-03-13  Matthias Clasen  <mclasen@redhat.com>
 
        * === Released 2.20.0 ===

diff --git a/gio/glocalfileoutputstream.c b/gio/glocalfileoutputstream.c
index 8ca3284..bd216df 100644 (file)
--- a/gio/glocalfileoutputstream.c
+++ b/gio/glocalfileoutputstream.c
@@ -69,6 +69,7 @@ struct _GLocalFileOutputStreamPrivate {
   char *original_filename;
   char *backup_filename;
   char *etag;
+  gboolean sync_on_close;
   int fd;
 };
 
@@ -190,6 +191,20 @@ g_local_file_output_stream_close (GOutputStream  *stream,
 
   file = G_LOCAL_FILE_OUTPUT_STREAM (stream);
 
+#ifdef HAVE_FSYNC
+  if (file->priv->sync_on_close &&
+      fsync (file->priv->fd) != 0)
+    {
+      int errsv = errno;
+      
+      g_set_error (error, G_IO_ERROR,
+                  g_io_error_from_errno (errno),
+                  _("Error writing to file: %s"),
+                  g_strerror (errsv));
+      goto err_out;
+    }
+#endif
+ 
 #ifdef G_OS_WIN32
 
   /* Must close before renaming on Windows, so just do the close first
@@ -976,6 +991,7 @@ _g_local_file_output_stream_replace (const char        *filename,
   int mode;
   int fd;
   char *temp_file;
+  gboolean sync_on_close;
 
   if (g_cancellable_set_error_if_cancelled (cancellable, error))
     return NULL;
@@ -986,6 +1002,7 @@ _g_local_file_output_stream_replace (const char        *filename,
     mode = 0600;
   else
     mode = 0666;
+  sync_on_close = FALSE;
 
   /* If the file doesn't exist, create it */
   fd = g_open (filename, O_CREAT | O_EXCL | O_WRONLY | O_BINARY, mode);
@@ -997,6 +1014,14 @@ _g_local_file_output_stream_replace (const char        *filename,
                                  flags, cancellable, error);
       if (fd == -1)
        return NULL;
+
+      /* If the final destination exists, we want to sync the newly written
+       * file to ensure the data is on disk when we rename over the destination.
+       * otherwise if we get a system crash we can lose both the new and the
+       * old file on some filesystems. (I.E. those that don't guarantee the
+       * data is written to the disk before the metadata.)
+       */
+      sync_on_close = TRUE;
     }
   else if (fd == -1)
     {
@@ -1022,6 +1047,7 @@ _g_local_file_output_stream_replace (const char        *filename,
  
   stream = g_object_new (G_TYPE_LOCAL_FILE_OUTPUT_STREAM, NULL);
   stream->priv->fd = fd;
+  stream->priv->sync_on_close = sync_on_close;
   stream->priv->tmp_filename = temp_file;
   if (create_backup)
     stream->priv->backup_filename = create_backup_filename (filename);

diff --git a/glib/gfileutils.c b/glib/gfileutils.c
index 547beff..4f5c669 100644 (file)
--- a/glib/gfileutils.c
+++ b/glib/gfileutils.c
@@ -868,7 +868,7 @@ rename_file (const char  *old_name,
 static gchar *
 write_to_temp_file (const gchar  *contents,
                    gssize        length,
-                   const gchar  *template,
+                   const gchar  *dest_file,
                    GError      **err)
 {
   gchar *tmp_name;
@@ -880,7 +880,7 @@ write_to_temp_file (const gchar  *contents,
 
   retval = NULL;
   
-  tmp_name = g_strdup_printf ("%s.XXXXXX", template);
+  tmp_name = g_strdup_printf ("%s.XXXXXX", dest_file);
 
   errno = 0;
   fd = create_temp_file (tmp_name, 0666);
@@ -942,11 +942,54 @@ write_to_temp_file (const gchar  *contents,
          goto out;
        }
     }
-   
+
+  errno = 0;
+  if (fflush (file) != 0)
+    { 
+      save_errno = errno;
+      
+      g_set_error (err,
+                  G_FILE_ERROR,
+                  g_file_error_from_errno (save_errno),
+                  _("Failed to write file '%s': fflush() failed: %s"),
+                  display_name, 
+                  g_strerror (save_errno));
+
+      g_unlink (tmp_name);
+      
+      goto out;
+    }
+  
+#ifdef HAVE_FSYNC
+  errno = 0;
+  /* If the final destination exists, we want to sync the newly written
+   * file to ensure the data is on disk when we rename over the destination.
+   * otherwise if we get a system crash we can lose both the new and the
+   * old file on some filesystems. (I.E. those that don't guarantee the
+   * data is written to the disk before the metadata.)
+   */
+  if (g_file_test (dest_file, G_FILE_TEST_EXISTS) &&
+      fsync (fileno (file)) != 0)
+    { 
+      save_errno = errno;
+      
+      g_set_error (err,
+                  G_FILE_ERROR,
+                  g_file_error_from_errno (save_errno),
+                  _("Failed to write file '%s': fsync() failed: %s"),
+                  display_name, 
+                  g_strerror (save_errno));
+
+      g_unlink (tmp_name);
+      
+      goto out;
+    }
+#endif
+  
   errno = 0;
   if (fclose (file) == EOF)
     { 
-      save_errno = 0;
+      save_errno = errno;
       
       g_set_error (err,
                   G_FILE_ERROR,