Repository locking 🔒

Makefile-libostree.am

@@ -261,6 +261,10 @@ OSTree-1.0.gir: libostree-1.la Makefile
 OSTree_1_0_gir_EXPORT_PACKAGES = ostree-1
 OSTree_1_0_gir_INCLUDES = Gio-2.0
 OSTree_1_0_gir_CFLAGS = $(libostree_1_la_CFLAGS)
+if ENABLE_EXPERIMENTAL_API
+# When compiling this is set via config.h, but g-ir-scanner doesn't use that
+OSTree_1_0_gir_CFLAGS += -DOSTREE_ENABLE_EXPERIMENTAL_API=1
+endif
 OSTree_1_0_gir_LIBS = libostree-1.la
 OSTree_1_0_gir_SCANNERFLAGS = --warn-all --identifier-prefix=Ostree --symbol-prefix=ostree $(GI_SCANNERFLAGS)
 OSTree_1_0_gir_FILES = $(libostreeinclude_HEADERS) $(filter-out %-private.h %/ostree-soup-uri.h $(libostree_experimental_headers),$(libostree_1_la_SOURCES))

Makefile-switchroot.am

@@ -65,7 +65,7 @@ systemdsystemgenerator_PROGRAMS = ostree-system-generator
 GITIGNOREFILES += $(systemdsystemgenerator_PROGRAMS)
 ostree_system_generator_SOURCES = src/switchroot/ostree-mount-util.h \
                                   src/switchroot/ostree-system-generator.c
-ostree_system_generator_CPPFLAGS = $(AM_CPPFLAGS) -I$(srcdir)/libglnx -I$(srcdir)/src/libostree
+ostree_system_generator_CPPFLAGS = $(AM_CPPFLAGS) -I$(srcdir)/libglnx -I$(srcdir)/src/libostree -I$(srcdir)/src/libotutil
 ostree_system_generator_CFLAGS = $(AM_CFLAGS) $(OT_INTERNAL_GIO_UNIX_CFLAGS)
 ostree_system_generator_LDADD = $(AM_LDFLAGS) libglnx.la libostree-1.la $(OT_INTERNAL_GIO_UNIX_LIBS)
 

Makefile-tests.am

@@ -35,6 +35,7 @@ AM_TESTS_ENVIRONMENT += OT_TESTS_DEBUG=1 \
 	LD_LIBRARY_PATH=$$(cd $(top_builddir)/.libs && pwd)$${LD_LIBRARY_PATH:+:$${LD_LIBRARY_PATH}} \
 	PATH=$$(cd $(top_builddir)/tests && pwd):$${PATH} \
 	OSTREE_FEATURES="$(OSTREE_FEATURES)" \
+	PYTHONUNBUFFERED=1 \
 	$(NULL)
 if BUILDOPT_ASAN
 AM_TESTS_ENVIRONMENT += OT_SKIP_READDIR_RAND=1 G_SLICE=always-malloc
@@ -106,6 +107,8 @@ _installed_or_uninstalled_test_scripts = \
 	tests/test-xattrs.sh \
 	tests/test-auto-summary.sh \
 	tests/test-prune.sh \
+	tests/test-concurrency.py \
+	tests/test-repo-locking.py \
 	tests/test-refs.sh \
 	tests/test-demo-buildsystem.sh \
 	tests/test-switchroot.sh \

apidoc/ostree-experimental-sections.txt

@@ -87,6 +87,13 @@ ostree_repo_finder_override_get_type
 
 <SECTION>
 <FILE>ostree-misc-experimental</FILE>
+ostree_repo_lock_push
+ostree_repo_lock_pop
+OstreeRepoAutoLock
+ostree_repo_auto_lock_push
+ostree_repo_auto_lock_cleanup
+ostree_repo_get_lock_timeout
+ostree_repo_set_lock_timeout
 ostree_repo_get_collection_id
 ostree_repo_set_collection_id
 ostree_validate_collection_id

man/ostree-admin.xml

@@ -105,6 +105,22 @@ Boston, MA 02111-1307, USA.
                   Prints the full path to the deployment directory for the currently active deployment in the specified sysroot to standard out. This is incompatible with specifying a subcommand.
                 </para></listitem>
             </varlistentry>
+
+            <varlistentry>
+              <term><option>--lock-timeout</option>=TIMEOUT</term>
+
+              <listitem>
+                <para>
+                  Some <command>ostree admin</command> commands require the
+                  repository to be locked while operating. This option sets
+                  the repository lock timeout to TIMEOUT seconds. If TIMEOUT
+                  is <literal>0</literal>, then one attempt to acquire or
+                  remove the lock will be made without waiting to try again.
+                  If TIMEOUT is <literal>-1</literal>, then the locking will
+                  block indefinitely. All other negative values are an error.
+                </para>
+              </listitem>
+            </varlistentry>
         </variablelist>
     </refsect1>
 </refentry>

man/ostree.repo-config.xml

@@ -143,6 +143,22 @@ Boston, MA 02111-1307, USA.
         </listitem>
       </varlistentry>
 
+      <varlistentry>
+        <term><varname>lock-timeout</varname></term>
+        <listitem>
+          <para>
+            Integer value controlling the default repository locking timeout
+            in seconds. This controls how long OSTree will wait attempting to
+            acquire or remove the repository lock. If the value is
+            <literal>0</literal>, then one attempt to acquire or remove the
+            lock will be made without waiting to try again. If the value is
+            <literal>-1</literal>, then ostree will block until the lock can
+            be acquired or removed. All other negative values are an error.
+            The default is <literal>30</literal> seconds.
+          </para>
+        </listitem>
+      </varlistentry>
+
     </variablelist>
   </refsect1>
 

man/ostree.xml

@@ -120,6 +120,22 @@ Boston, MA 02111-1307, USA.
                 </para></listitem>
             </varlistentry>
 
+            <varlistentry>
+              <term><option>--lock-timeout</option>=TIMEOUT</term>
+
+              <listitem>
+                <para>
+                  Some <command>ostree</command> commands require the
+                  repository to be locked while operating. This option sets
+                  the repository lock timeout to TIMEOUT seconds. If TIMEOUT
+                  is <literal>0</literal>, then one attempt to acquire or
+                  remove the lock will be made without waiting to try again.
+                  If TIMEOUT is <literal>-1</literal>, then the locking will
+                  block indefinitely. All other negative values are an error.
+                </para>
+              </listitem>
+            </varlistentry>
+
             <varlistentry>
                 <term><option>-v, --verbose</option></term>
 

src/libostree/libostree-experimental.sym

@@ -88,4 +88,10 @@ global:
   ostree_repo_finder_override_add_uri;
   ostree_repo_finder_override_get_type;
   ostree_repo_finder_override_new;
+  ostree_repo_auto_lock_cleanup;
+  ostree_repo_auto_lock_push;
+  ostree_repo_get_lock_timeout;
+  ostree_repo_lock_pop;
+  ostree_repo_lock_push;
+  ostree_repo_set_lock_timeout;
 } LIBOSTREE_2017.12_EXPERIMENTAL;

src/libostree/ostree-autocleanups.h

@@ -59,6 +59,7 @@ G_DEFINE_AUTOPTR_CLEANUP_FUNC (OstreeSysrootUpgrader, g_object_unref)
 G_DEFINE_AUTO_CLEANUP_CLEAR_FUNC (OstreeRepoCommitTraverseIter, ostree_repo_commit_traverse_iter_clear)
 
 #ifdef OSTREE_ENABLE_EXPERIMENTAL_API
+G_DEFINE_AUTOPTR_CLEANUP_FUNC (OstreeRepoAutoLock, ostree_repo_auto_lock_cleanup)
 G_DEFINE_AUTOPTR_CLEANUP_FUNC (OstreeCollectionRef, ostree_collection_ref_free)
 G_DEFINE_AUTO_CLEANUP_FREE_FUNC (OstreeCollectionRefv, ostree_collection_ref_freev, NULL)
 G_DEFINE_AUTOPTR_CLEANUP_FUNC (OstreeRemote, ostree_remote_unref)

src/libostree/ostree-cmdprivate.c

@@ -1,3 +1,4 @@
+
 /*
  * Copyright (C) 2014 Colin Walters <[email protected]>
  *
@@ -35,7 +36,7 @@ impl_ostree_generate_grub2_config (OstreeSysroot *sysroot, int bootversion, int
 }
 
 /**
- * ostree_cmdprivate: (skip)
+ * ostree_cmd__private__: (skip)
  *
  * Do not call this function; it is used to share private API between
  * the OSTree commandline and the library.
@@ -48,7 +49,14 @@ ostree_cmd__private__ (void)
     impl_ostree_generate_grub2_config,
     _ostree_repo_static_delta_dump,
     _ostree_repo_static_delta_query_exists,
-    _ostree_repo_static_delta_delete
+    _ostree_repo_static_delta_delete,
+    /* Remove this when ostree_repo_set_lock_timeout is no longer experimental */
+    ostree_repo_set_lock_timeout,
+    /* Remove these when ostree_repo_auto_lock_push and
+     * ostree_repo_auto_lock_cleanup are no longer experimental
+     */
+    ostree_repo_auto_lock_push,
+    ostree_repo_auto_lock_cleanup,
   };
 
   return &table;

src/libostree/ostree-cmdprivate.h

@@ -20,17 +20,34 @@
 #pragma once
 
 #include "ostree-types.h"
+/* This is only needed to get the OstreeRepoLockType and OstreeRepoAutoLock
+ * types. This should be removed once they're no longer experimental.
+ */
+#include "ostree-repo-private.h"
 
 G_BEGIN_DECLS
 
 gboolean _ostree_impl_system_generator (const char *ostree_cmdline, const char *normal_dir, const char *early_dir, const char *late_dir, GError **error);
 
+/**
+ * OstreeCmdPrivateVTable: (skip)
+ *
+ * Function table to share private API between the OSTree commandline and the
+ * library. Don't use this.
+ */
 typedef struct {
   gboolean (* ostree_system_generator) (const char *ostree_cmdline, const char *normal_dir, const char *early_dir, const char *late_dir, GError **error);
   gboolean (* ostree_generate_grub2_config) (OstreeSysroot *sysroot, int bootversion, int target_fd, GCancellable *cancellable, GError **error);
   gboolean (* ostree_static_delta_dump) (OstreeRepo *repo, const char *delta_id, GCancellable *cancellable, GError **error);
   gboolean (* ostree_static_delta_query_exists) (OstreeRepo *repo, const char *delta_id, gboolean *out_exists, GCancellable *cancellable, GError **error);
   gboolean (* ostree_static_delta_delete) (OstreeRepo *repo, const char *delta_id, GCancellable *cancellable, GError **error);
+  /* Remove this when ostree_repo_set_lock_timeout is no longer experimental */
+  gboolean (* ostree_repo_set_lock_timeout) (OstreeRepo *repo, gint timeout);
+  /* Remove these when ostree_repo_auto_lock_push and
+   * ostree_repo_auto_lock_cleanup are no longer experimental
+   */
+  OstreeRepoAutoLock * (* ostree_repo_auto_lock_push) (OstreeRepo *repo, OstreeRepoLockType lock_type, GCancellable *cancellable, GError **error);
+  void (* ostree_repo_auto_lock_cleanup) (OstreeRepoAutoLock *lock);
 } OstreeCmdPrivateVTable;
 
 /* Note this not really "public", we just export the symbol, but not the header */

src/libostree/ostree-repo-checkout.c

@@ -31,6 +31,7 @@
 #include "ostree-sepolicy-private.h"
 #include "ostree-core-private.h"
 #include "ostree-repo-private.h"
+#include "ostree-autocleanups.h"
 
 #define WHITEOUT_PREFIX ".wh."
 
@@ -991,6 +992,16 @@ checkout_tree_at (OstreeRepo                        *self,
       g_assert (options->force_copy);
     }
 
+  /* Take a shared repo lock to try to ensure objects aren't deleted. If the
+   * repo is not writable, this will be a noop and we just hope for the
+   * best...
+   */
+  g_autoptr(OstreeRepoAutoLock) lock = NULL;
+  lock = ostree_repo_auto_lock_push (self, OSTREE_REPO_LOCK_SHARED,
+                                     cancellable, error);
+  if (!lock)
+    return FALSE;
+
   /* Special case handling for subpath of a non-directory */
   if (g_file_info_get_file_type (source_info) != G_FILE_TYPE_DIRECTORY)
     {
@@ -1062,6 +1073,8 @@ canonicalize_options (OstreeRepo                  *self,
  * physical filesystem.  @source may be any subdirectory of a given
  * commit.  The @mode and @overwrite_mode allow control over how the
  * files are checked out.
+ *
+ * This function takes a shared lock on the @self repository.
  */
 gboolean
 ostree_repo_checkout_tree (OstreeRepo               *self,
@@ -1105,6 +1118,8 @@ ostree_repo_checkout_tree (OstreeRepo               *self,
  * default is not to use the repository-internal uncompressed objects
  * cache.
  *
+ * This function takes a shared lock on the @self repository.
+ *
  * This function is deprecated.  Use ostree_repo_checkout_at() instead.
  */
 gboolean
@@ -1150,6 +1165,8 @@ ostree_repo_checkout_tree_at (OstreeRepo                        *self,
  * Note in addition that unlike ostree_repo_checkout_tree(), the
  * default is not to use the repository-internal uncompressed objects
  * cache.
+ *
+ * This function takes a shared lock on the @self repository.
  */
 gboolean
 ostree_repo_checkout_at (OstreeRepo                        *self,
@@ -1273,12 +1290,21 @@ ostree_repo_devino_cache_new (void)
  * Call this after finishing a succession of checkout operations; it
  * will delete any currently-unused uncompressed objects from the
  * cache.
+ *
+ * This function takes an exclusive lock on the @self repository.
  */
 gboolean
 ostree_repo_checkout_gc (OstreeRepo        *self,
                          GCancellable      *cancellable,
                          GError           **error)
 {
+  /* Take an exclusive repo lock while deleting uncompressed objects */
+  g_autoptr(OstreeRepoAutoLock) lock = NULL;
+  lock = ostree_repo_auto_lock_push (self, OSTREE_REPO_LOCK_EXCLUSIVE,
+                                     cancellable, error);
+  if (!lock)
+    return FALSE;
+
   g_autoptr(GHashTable) to_clean_dirs = NULL;
 
   g_mutex_lock (&self->cache_lock);

src/libostree/ostree-repo-commit.c

@@ -986,13 +986,16 @@ write_metadata_object (OstreeRepo         *self,
   if (objtype == OSTREE_OBJECT_TYPE_COMMIT)
     {
       GError *local_error = NULL;
-      /* If we are writing a commit, be sure there is no tombstone for it.
-         We may have deleted the commit and now we are trying to pull it again.  */
-      if (!ostree_repo_delete_object (self,
-                                      OSTREE_OBJECT_TYPE_TOMBSTONE_COMMIT,
-                                      actual_checksum,
-                                      cancellable,
-                                      &local_error))
+      /* If we are writing a commit, be sure there is no tombstone for it. We
+       * may have deleted the commit and now we are trying to pull it again.
+       * Delete with a shared lock here so concurrent commits and pulls aren't
+       * prevented.
+       */
+      if (!_ostree_repo_delete_object_shared (self,
+                                              OSTREE_OBJECT_TYPE_TOMBSTONE_COMMIT,
+                                              actual_checksum,
+                                              cancellable,
+                                              &local_error))
         {
           if (g_error_matches (local_error, G_IO_ERROR, G_IO_ERROR_NOT_FOUND))
             g_clear_error (&local_error);
@@ -1203,7 +1206,9 @@ ostree_repo_scan_hardlinks (OstreeRepo    *self,
  * ostree_repo_abort_transaction().
  *
  * Currently, transactions are not atomic, and aborting a transaction
- * will not erase any data you  write during the transaction.
+ * will not erase any data you write during the transaction.
+ *
+ * This function takes a shared lock on the @self repository.
  */
 gboolean
 ostree_repo_prepare_transaction (OstreeRepo     *self,
@@ -1214,6 +1219,11 @@ ostree_repo_prepare_transaction (OstreeRepo     *self,
 
   g_return_val_if_fail (self->in_transaction == FALSE, FALSE);
 
+  self->txn_locked = ostree_repo_lock_push (self, OSTREE_REPO_LOCK_SHARED,
+                                            cancellable, error);
+  if (!self->txn_locked)
+    return FALSE;
+
   memset (&self->txn_stats, 0, sizeof (OstreeRepoTransactionStats));
 
   self->in_transaction = TRUE;
@@ -1699,6 +1709,13 @@ ostree_repo_commit_transaction (OstreeRepo                  *self,
   if (!ot_ensure_unlinked_at (self->repo_dir_fd, "transaction", 0))
     return FALSE;
 
+  if (self->txn_locked)
+    {
+      if (!ostree_repo_lock_pop (self, cancellable, error))
+        return FALSE;
+      self->txn_locked = FALSE;
+    }
+
   if (out_stats)
     *out_stats = self->txn_stats;
 
@@ -1739,6 +1756,13 @@ ostree_repo_abort_transaction (OstreeRepo     *self,
 
   self->in_transaction = FALSE;
 
+  if (self->txn_locked)
+    {
+      if (!ostree_repo_lock_pop (self, cancellable, error))
+        return FALSE;
+      self->txn_locked = FALSE;
+    }
+
   return TRUE;
 }
 
@@ -2355,6 +2379,8 @@ ostree_repo_read_commit_detached_metadata (OstreeRepo      *self,
  * Replace any existing metadata associated with commit referred to by
  * @checksum with @metadata.  If @metadata is %NULL, then existing
  * data will be deleted.
+ *
+ * This function takes a shared lock on the @self repository.
  */
 gboolean
 ostree_repo_write_commit_detached_metadata (OstreeRepo      *self,
@@ -2363,6 +2389,13 @@ ostree_repo_write_commit_detached_metadata (OstreeRepo      *self,
                                             GCancellable    *cancellable,
                                             GError         **error)
 {
+  /* Take shared lock so associated commit doesn't get deleted */
+  g_autoptr(OstreeRepoAutoLock) lock = NULL;
+  lock = ostree_repo_auto_lock_push (self, OSTREE_REPO_LOCK_SHARED,
+                                     cancellable, error);
+  if (!lock)
+    return FALSE;
+
   int dest_dfd;
   if (self->in_transaction)
     dest_dfd = self->commit_stagedir.fd;