From 6cff88ba18b3bc0d118308f109840cb163dcea03 Mon Sep 17 00:00:00 2001 From: Alexander Larsson Date: Mon, 16 Mar 2009 16:03:13 +0000 Subject: [PATCH] =?utf8?q?Bug=20575555=20=E2=80=93=20Use=20fsync()=20when?= =?utf8?q?=20replacing=20files=20to=20avoid=20data=20loss=20on?= MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit 2009-03-16 Alexander Larsson 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 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 --- ChangeLog | 11 ++++++++ configure.in | 1 + gio/ChangeLog | 9 +++++++ gio/glocalfileoutputstream.c | 26 ++++++++++++++++++ glib/gfileutils.c | 51 +++++++++++++++++++++++++++++++++--- 5 files changed, 94 insertions(+), 4 deletions(-) diff --git a/ChangeLog b/ChangeLog index 9339cf94..a97312a7 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,14 @@ +2009-03-16 Alexander Larsson + + 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 * configure.in: Bump version diff --git a/configure.in b/configure.in index 186fd21c..9922b50a 100644 --- 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 604e38d7..8f734816 100644 --- a/gio/ChangeLog +++ b/gio/ChangeLog @@ -1,3 +1,12 @@ +2009-03-16 Alexander Larsson + + 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 * === Released 2.20.0 === diff --git a/gio/glocalfileoutputstream.c b/gio/glocalfileoutputstream.c index 8ca32842..bd216dfb 100644 --- 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 547beff4..4f5c669b 100644 --- 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, -- 2.34.1