lib/repo-commit: Delay propagation of errors from abort_transaction()
authorPhilip Withnall <withnall@endlessm.com>
Thu, 14 Jun 2018 15:47:43 +0000 (16:47 +0100)
committerAtomic Bot <atomic-devel@projectatomic.io>
Thu, 14 Jun 2018 17:13:43 +0000 (17:13 +0000)
If there’s a problem while aborting a transaction, store the error but
don’t report it until the end of the function — do a best effort at
clearing the rest of the transaction state first (since most of it
cannot fail).

If cleanup_tmpdir() fails (which, arguably, should not be a
showstopper), this allows a caller to recover and start a new
transaction in future.

Signed-off-by: Philip Withnall <withnall@endlessm.com>
Closes: #1626
Approved by: jlebon

src/libostree/ostree-repo-commit.c

index 06ade885a88a940b38b6c71b109d48e122c4e5cb..be5e55cdfdce2da325f82b5a50425efeeb43e89d 100644 (file)
@@ -2188,6 +2188,8 @@ ostree_repo_abort_transaction (OstreeRepo     *self,
                                GCancellable   *cancellable,
                                GError        **error)
 {
+  g_autoptr(GError) cleanup_error = NULL;
+
   /* Always ignore the cancellable to avoid the chance that, if it gets
    * canceled, the transaction may not be fully cleaned up.
    * See https://github.com/ostreedev/ostree/issues/1491 .
@@ -2198,8 +2200,9 @@ ostree_repo_abort_transaction (OstreeRepo     *self,
   if (!self->in_transaction)
     return TRUE;
 
-  if (!cleanup_tmpdir (self, cancellable, error))
-    return FALSE;
+  /* Do not propagate failures from cleanup_tmpdir() immediately, as we want
+   * to clean up the rest of the internal transaction state first. */
+  cleanup_tmpdir (self, cancellable, &cleanup_error);
 
   if (self->loose_object_devino_hash)
     g_hash_table_remove_all (self->loose_object_devino_hash);
@@ -2219,6 +2222,13 @@ ostree_repo_abort_transaction (OstreeRepo     *self,
       self->txn_locked = FALSE;
     }
 
+  /* Propagate cleanup_tmpdir() failure. */
+  if (cleanup_error != NULL)
+    {
+      g_propagate_error (error, g_steal_pointer (&cleanup_error));
+      return FALSE;
+    }
+
   return TRUE;
 }