Skip to content

Commit

Permalink
repo: Add a file lock to protect operations against concurrent prune ops
Browse files Browse the repository at this point in the history
In general, we're pretty robust when there are multiple ongoing operations,
because transactions are isolated to per-transaction staging dirs, and the
transaction commit is a last-writer-win replace operation on immutable data.

However, any non-atomic read operation can fail if it is concurrent with a
prune, as then objects may disappear under the operations feet half-way.

This patch-set makes this robust by having a repo lock that is taken in a shared
fashion for all transactions, and for checkouts, but is taken in exclusive mode
for prune. It also adds a non-blocking prune mode that allows you to do
opportunistic prunes that don't block if there is an ongoing operation.

There are a bunch of operations that are still unsafe (does not block prune),
such as: cat, ls, show, log, diff, generate-delta, rev-parse. I don't think that
is necessarily a huge problem, as these are mainly used in development or
debugging, and a failure here will just be an error printed, it will never cause
a broken repo.

This is a simpler version of what i proposed at:
https://mail.gnome.org/archives/ostree-list/2015-December/msg00024.html - it
takes a lock during the whole transaction rather than retrying the transaction
under a lock when commiting it.

https://bugzilla.gnome.org/show_bug.cgi?id=759442
  • Loading branch information
alexlarsson authored and cgwalters committed Apr 26, 2017
1 parent 480e0ac commit 7a1f9fd
Show file tree
Hide file tree
Showing 6 changed files with 83 additions and 3 deletions.
8 changes: 8 additions & 0 deletions src/libostree/ostree-repo-checkout.c
Original file line number Diff line number Diff line change
Expand Up @@ -800,6 +800,10 @@ checkout_tree_at (OstreeRepo *self,
g_assert (options->force_copy);
}

/* And grab the repo checkout lock */
g_auto(GLnxLockFile) repo_lock = GLNX_LOCK_FILE_INIT;
if (!_ostree_repo_lock_shared (self, FALSE, &repo_lock, error))
return FALSE;
return checkout_tree_at_recurse (self, options, &state, destination_parent_fd,
destination_name,
source, source_info,
Expand Down Expand Up @@ -1035,6 +1039,10 @@ ostree_repo_checkout_gc (OstreeRepo *self,
GError **error)
{
g_autoptr(GHashTable) to_clean_dirs = NULL;
g_auto(GLnxLockFile) repo_lock = GLNX_LOCK_FILE_INIT;

if (!_ostree_repo_lock_exclusive (self, FALSE, &repo_lock, error))
return FALSE;

g_mutex_lock (&self->cache_lock);
to_clean_dirs = self->updated_uncompressed_dirs;
Expand Down
8 changes: 8 additions & 0 deletions src/libostree/ostree-repo-commit.c
Original file line number Diff line number Diff line change
Expand Up @@ -1113,6 +1113,10 @@ ostree_repo_prepare_transaction (OstreeRepo *self,

g_return_val_if_fail (self->in_transaction == FALSE, FALSE);

if (!_ostree_repo_lock_shared (self, FALSE,
&self->transaction_repo_lock, error))
goto out;

memset (&self->txn_stats, 0, sizeof (OstreeRepoTransactionStats));

self->in_transaction = TRUE;
Expand Down Expand Up @@ -1477,6 +1481,8 @@ ostree_repo_commit_transaction (OstreeRepo *self,
if (!ot_ensure_unlinked_at (self->repo_dir_fd, "transaction", 0))
return FALSE;

glnx_release_lock_file (&self->transaction_repo_lock);

if (out_stats)
*out_stats = self->txn_stats;

Expand Down Expand Up @@ -1509,6 +1515,8 @@ ostree_repo_abort_transaction (OstreeRepo *self,
}
g_clear_pointer (&self->commit_stagedir_name, g_free);

glnx_release_lock_file (&self->transaction_repo_lock);

self->in_transaction = FALSE;

return TRUE;
Expand Down
13 changes: 13 additions & 0 deletions src/libostree/ostree-repo-private.h
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,8 @@ struct OstreeRepo {
GFile *sysroot_dir;
char *remotes_config_dir;

GFile *transaction_lock_path;
GLnxLockFile transaction_repo_lock;
GHashTable *txn_refs;
GMutex txn_stats_lock;
OstreeRepoTransactionStats txn_stats;
Expand Down Expand Up @@ -343,4 +345,15 @@ gboolean
_ostree_repo_update_mtime (OstreeRepo *self,
GError **error);

gboolean
_ostree_repo_lock_shared (OstreeRepo *self,
gboolean non_blocking,
GLnxLockFile *lockfile,
GError **error);

gboolean
_ostree_repo_lock_exclusive (OstreeRepo *self,
gboolean non_blocking,
GLnxLockFile *lockfile,
GError **error);
G_END_DECLS
11 changes: 11 additions & 0 deletions src/libostree/ostree-repo-prune.c
Original file line number Diff line number Diff line change
Expand Up @@ -350,6 +350,12 @@ ostree_repo_prune (OstreeRepo *self,
g_autoptr(GHashTable) all_refs = NULL;
g_autoptr(GHashTable) reachable = NULL;
gboolean refs_only = flags & OSTREE_REPO_PRUNE_FLAGS_REFS_ONLY;
gboolean non_blocking = flags & OSTREE_REPO_PRUNE_FLAGS_NON_BLOCKING;
g_auto(GLnxLockFile) repo_lock = GLNX_LOCK_FILE_INIT;

if (!_ostree_repo_lock_exclusive (self, non_blocking,
&repo_lock, error))
return FALSE;

reachable = ostree_repo_traverse_new_reachable ();

Expand Down Expand Up @@ -441,6 +447,11 @@ ostree_repo_prune_from_reachable (OstreeRepo *self,
GError **error)
{
g_autoptr(GHashTable) objects = NULL;
g_auto(GLnxLockFile) repo_lock = GLNX_LOCK_FILE_INIT;

if (!_ostree_repo_lock_exclusive (self, non_blocking,
&repo_lock, error))
return FALSE;

if (!ostree_repo_list_objects (self, OSTREE_REPO_LIST_OBJECTS_ALL | OSTREE_REPO_LIST_OBJECTS_NO_PARENTS,
&objects, cancellable, error))
Expand Down
39 changes: 39 additions & 0 deletions src/libostree/ostree-repo.c
Original file line number Diff line number Diff line change
Expand Up @@ -528,6 +528,8 @@ ostree_repo_finalize (GObject *object)
(void) close (self->uncompressed_objects_dir_fd);
g_clear_object (&self->sysroot_dir);
g_free (self->remotes_config_dir);
g_clear_object (&self->transaction_lock_path);
glnx_release_lock_file (&self->transaction_repo_lock);

if (self->loose_object_devino_hash)
g_hash_table_destroy (self->loose_object_devino_hash);
Expand Down Expand Up @@ -705,6 +707,7 @@ ostree_repo_init (OstreeRepo *self)
self->objects_dir_fd = -1;
self->uncompressed_objects_dir_fd = -1;
self->commit_stagedir_lock = empty_lockfile;
self->transaction_repo_lock = empty_lockfile;
}

/**
Expand Down Expand Up @@ -4895,3 +4898,39 @@ _ostree_repo_memory_cache_ref_destroy (OstreeRepoMemoryCacheRef *state)
g_mutex_unlock (lock);
g_object_unref (repo);
}

gboolean
_ostree_repo_lock_shared (OstreeRepo *self,
gboolean non_blocking,
GLnxLockFile *lockfile,
GError **error)
{
int lock_op = LOCK_SH;

if (non_blocking)
lock_op |= LOCK_NB;

if (!glnx_make_lock_file (self->repo_dir_fd, "lock", lock_op,
lockfile, error))
return FALSE;

return TRUE;
}

gboolean
_ostree_repo_lock_exclusive (OstreeRepo *self,
gboolean non_blocking,
GLnxLockFile *lockfile,
GError **error)
{
int lock_op = LOCK_EX;

if (non_blocking)
lock_op |= LOCK_NB;

if (!glnx_make_lock_file (self->repo_dir_fd, "lock", lock_op,
lockfile, error))
return FALSE;

return TRUE;
}
7 changes: 4 additions & 3 deletions src/libostree/ostree-repo.h
Original file line number Diff line number Diff line change
Expand Up @@ -967,9 +967,10 @@ void ostree_repo_commit_traverse_iter_cleanup (void *p);
* @OSTREE_REPO_PRUNE_FLAGS_REFS_ONLY: Do not traverse individual commit objects, only follow refs
*/
typedef enum {
OSTREE_REPO_PRUNE_FLAGS_NONE,
OSTREE_REPO_PRUNE_FLAGS_NO_PRUNE,
OSTREE_REPO_PRUNE_FLAGS_REFS_ONLY
OSTREE_REPO_PRUNE_FLAGS_NONE = 0,
OSTREE_REPO_PRUNE_FLAGS_NO_PRUNE = (1 << 0),
OSTREE_REPO_PRUNE_FLAGS_REFS_ONLY = (1 << 1),
OSTREE_REPO_PRUNE_FLAGS_NON_BLOCKING = (1 << 2)
} OstreeRepoPruneFlags;

_OSTREE_PUBLIC
Expand Down

0 comments on commit 7a1f9fd

Please sign in to comment.