Skip to content

Commit

Permalink
repo: Make locking per-repo structure
Browse files Browse the repository at this point in the history 
Previously each thread maintained its own lock file descriptor
regardless of whether the thread was using the same `OstreeRepo` as
another thread. This was very safe but it made certain multithreaded
procedures difficult. For example, if a main thread took an exclusive
lock and then spawned worker threads, it would deadlock if one of the
worker threads tried to acquire the lock.

This moves the file descriptor from thread local storage to the
`OstreeRepo` structure so that threads using the same `OstreeRepo` can
share the lock. Some bookkeeping is needed to keep track of the lock
state when recursing and a mutex guards against threads altering the
state concurrently.

The required GLib is bumped to 2.44 to allow usage of `GMutexLocker`.

Fixes: ostreedev#2344
  • Loading branch information
@dbnicholson
dbnicholson committed Apr 16, 2021
1 parent df30d38 commit b9f0a49
Show file tree
Hide file tree
Showing 2 changed files with 113 additions and 72 deletions.
9 changes: 9 additions & 0 deletions src/libostree/ostree-repo-private.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,13 @@ typedef struct {
fsblkcnt_t max_blocks;
} OstreeRepoTxn;

typedef struct {
GMutex mutex;
int fd;
guint shared; /* Number of shared locks */
guint exclusive; /* Number of exclusive locks */
} OstreeRepoLock;

typedef enum {
_OSTREE_FEATURE_NO,
_OSTREE_FEATURE_MAYBE,
Expand Down Expand Up @@ -159,6 +166,8 @@ struct OstreeRepo {
GWeakRef sysroot; /* Weak to avoid a circular ref; see also `is_system` */
char *remotes_config_dir;

OstreeRepoLock lock;

GMutex txn_lock;
OstreeRepoTxn txn;
gboolean txn_locked;
Expand Down
176 changes: 104 additions & 72 deletions src/libostree/ostree-repo.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -172,27 +172,38 @@ G_DEFINE_TYPE (OstreeRepo, ostree_repo, G_TYPE_OBJECT)
/* Repository locking
*
* To guard against objects being deleted (e.g., prune) while they're in
* use by another operation is accessing them (e.g., commit), the
* use by another operation that is accessing them (e.g., commit), the
* repository must be locked by concurrent writers.
*
* The locking is implemented by maintaining a thread local table of
* lock stacks per repository. This allows thread safe locking since
* each thread maintains its own lock stack. See the OstreeRepoLock type
* below.
* The repository locking has several important features:
*
* The actual locking is done using either open file descriptor locks or
* flock locks. This allows the locking to work with concurrent
* processes. The lock file is held on the ".lock" file within the
* repository.
* * There are 2 states - shared and exclusive. Multiple users can hold
* a shared lock concurrently while only one user can hold an
* exclusive lock.
*
* * The lock can be taken recursively so long as each acquisition is
* paired with a matching release. The recursion is also latched to
* the strongest state. Once an exclusive lock has been taken, it will
* remain exclusive until all exclusive states have been released.
*
* * It is both multiprocess- and multithread-safe. Threads that share
* an OstreeRepo use the lock cooperatively while processes and
* threads using separate OstreeRepo structures will block when
* acquiring incompatible lock states.
*
* The actual locking is implemented using either open file descriptor
* locks or flock locks. This allows the locking to work with concurrent
* processes or concurrent threads using a separate OstreeRepo. The lock
* file is held on the ".lock" file within the repository.
*
* The intended usage is to take a shared lock when writing objects or
* reading objects in critical sections. Exclusive locks are taken when
* deleting objects.
*
* To allow fine grained locking within libostree, the lock is
* maintained as a stack. The core APIs then push or pop from the stack.
* When pushing or popping a lock state identical to the existing or
* next state, the stack is simply updated. Only when upgrading or
* To allow fine grained locking within libostree, the lock is treated
* as a stack. The core APIs then push or pop from the stack. When
* pushing or popping a lock state identical to the existing or next
* state, the lock state is simply updated. Only when upgrading or
* downgrading the lock (changing to/from unlocked, pushing exclusive on
* shared or popping exclusive to shared) are actual locking operations
* performed.
Expand All @@ -210,62 +221,51 @@ free_repo_lock_table (gpointer data)
}
}

/* When pushing lock states, it's clearly known what the current and next lock
* state are. However, when popping lock states we need to what the lock state
* that was pushed by the current thread was so we know what state to drop. A
* stack is kept per thread for this.
*
* In order to allow a single thread to have multiple OstreeRepos, a hash
* table with the OstreeRepo as key and GPtrArray as value is maintained in
* thread local storage via GPrivate. The array represents that thread's lock
* stack for a particular OstreeRepo.
*/
static GPrivate repo_lock_table = G_PRIVATE_INIT (free_repo_lock_table);

typedef struct {
int fd;
GQueue stack;
} OstreeRepoLock;

typedef struct {
guint len;
int state;
const char *name;
} OstreeRepoLockInfo;

static void
repo_lock_info (OstreeRepoLock *lock, OstreeRepoLockInfo *out_info)
repo_lock_info (OstreeRepo *self, OstreeRepoLockInfo *out_info)
{
g_assert (lock != NULL);
g_assert (self != NULL);
g_assert (out_info != NULL);

OstreeRepoLockInfo info;
info.len = g_queue_get_length (&lock->stack);
info.len = self->lock.shared + self->lock.exclusive;
if (info.len == 0)
{
info.state = LOCK_UN;
info.name = "unlocked";
}
else if (self->lock.exclusive > 0)
{
info.state = LOCK_EX;
info.name = "exclusive";
}
else
{
info.state = GPOINTER_TO_INT (g_queue_peek_head (&lock->stack));
info.name = (info.state == LOCK_EX) ? "exclusive" : "shared";
info.state = LOCK_SH;
info.name = "shared";
}

*out_info = info;
}

static void
free_repo_lock (gpointer data)
{
OstreeRepoLock *lock = data;

if (lock != NULL)
{
OstreeRepoLockInfo info;
repo_lock_info (lock, &info);

g_debug ("Free lock: state=%s, depth=%u", info.name, info.len);
g_queue_clear (&lock->stack);
if (lock->fd >= 0)
{
g_debug ("Closing repo lock file");
(void) close (lock->fd);
}
g_free (lock);
}
}

/* Wrapper to handle flock vs OFD locking based on GLnxLockFile */
static gboolean
do_repo_lock (int fd,
Expand Down Expand Up @@ -347,49 +347,56 @@ push_repo_lock (OstreeRepo *self,
{
g_debug ("Creating repo lock table");
lock_table = g_hash_table_new_full (NULL, NULL, NULL,
(GDestroyNotify)free_repo_lock);
(GDestroyNotify)g_ptr_array_unref);
g_private_set (&repo_lock_table, lock_table);
}

OstreeRepoLock *lock = g_hash_table_lookup (lock_table, self);
if (lock == NULL)
GPtrArray *stack = g_hash_table_lookup (lock_table, self);
if (stack == NULL)
{
stack = g_ptr_array_new ();
g_hash_table_insert (lock_table, self, stack);
}

g_autoptr(GMutexLocker) locker = g_mutex_locker_new (&self->lock.mutex);

if (self->lock.fd == -1)
{
lock = g_new0 (OstreeRepoLock, 1);
g_queue_init (&lock->stack);
g_debug ("Opening repo lock file");
lock->fd = TEMP_FAILURE_RETRY (openat (self->repo_dir_fd, ".lock",
O_CREAT | O_RDWR | O_CLOEXEC,
DEFAULT_REGFILE_MODE));
if (lock->fd < 0)
{
free_repo_lock (lock);
return glnx_throw_errno_prefix (error,
"Opening lock file %s/.lock failed",
gs_file_get_path_cached (self->repodir));
}
g_hash_table_insert (lock_table, self, lock);
self->lock.fd = TEMP_FAILURE_RETRY (openat (self->repo_dir_fd, ".lock",
O_CREAT | O_RDWR | O_CLOEXEC,
DEFAULT_REGFILE_MODE));
if (self->lock.fd < 0)
return glnx_throw_errno_prefix (error,
"Opening lock file %s/.lock failed",
gs_file_get_path_cached (self->repodir));
}

OstreeRepoLockInfo info;
repo_lock_info (lock, &info);
repo_lock_info (self, &info);
g_debug ("Push lock: state=%s, depth=%u", info.name, info.len);

if (info.state == LOCK_EX)
{
g_debug ("Repo already locked exclusively, extending stack");
g_queue_push_head (&lock->stack, GINT_TO_POINTER (LOCK_EX));
self->lock.exclusive++;
g_ptr_array_add (stack, GINT_TO_POINTER (LOCK_EX));
}
else
{
int next_state = (flags & LOCK_EX) ? LOCK_EX : LOCK_SH;
const char *next_state_name = (flags & LOCK_EX) ? "exclusive" : "shared";

g_debug ("Locking repo %s", next_state_name);
if (!do_repo_lock (lock->fd, flags))
if (!do_repo_lock (self->lock.fd, flags))
return glnx_throw_errno_prefix (error, "Locking repo %s failed",
next_state_name);

g_queue_push_head (&lock->stack, GINT_TO_POINTER (next_state));
if (next_state == LOCK_EX)
self->lock.exclusive++;
else
self->lock.shared++;
g_ptr_array_add (stack, GINT_TO_POINTER (next_state));
}

return TRUE;
Expand All @@ -405,26 +412,40 @@ pop_repo_lock (OstreeRepo *self,
GHashTable *lock_table = g_private_get (&repo_lock_table);
g_return_val_if_fail (lock_table != NULL, FALSE);

OstreeRepoLock *lock = g_hash_table_lookup (lock_table, self);
g_return_val_if_fail (lock != NULL, FALSE);
g_return_val_if_fail (lock->fd != -1, FALSE);
GPtrArray *stack = g_hash_table_lookup (lock_table, self);
g_return_val_if_fail (stack != NULL, FALSE);

g_autoptr(GMutexLocker) locker = g_mutex_locker_new (&self->lock.mutex);
g_return_val_if_fail (self->lock.fd != -1, FALSE);

OstreeRepoLockInfo info;
repo_lock_info (lock, &info);
repo_lock_info (self, &info);
g_return_val_if_fail (info.len > 0, FALSE);

g_debug ("Pop lock: state=%s, depth=%u", info.name, info.len);
int state_to_drop = GPOINTER_TO_INT (stack->pdata[stack->len - 1]);
if (info.len > 1)
{
int next_state = GPOINTER_TO_INT (g_queue_peek_nth (&lock->stack, 1));
int next_state;

if (state_to_drop == LOCK_EX)
{
g_return_val_if_fail (self->lock.exclusive > 0, FALSE);
next_state = (self->lock.exclusive > 1) ? LOCK_EX : LOCK_SH;
}
else
{
g_return_val_if_fail (self->lock.shared > 0, FALSE);
next_state = (self->lock.exclusive > 0) ? LOCK_EX : LOCK_SH;
}

/* Drop back to the previous lock state if it differs */
if (next_state != info.state)
{
/* We should never drop from shared to exclusive */
g_return_val_if_fail (next_state == LOCK_SH, FALSE);
g_debug ("Returning lock state to shared");
if (!do_repo_lock (lock->fd, next_state | flags))
if (!do_repo_lock (self->lock.fd, next_state | flags))
return glnx_throw_errno_prefix (error,
"Setting repo lock to shared failed");
}
Expand All @@ -435,11 +456,15 @@ pop_repo_lock (OstreeRepo *self,
{
/* Lock stack will be empty, unlock */
g_debug ("Unlocking repo");
if (!do_repo_unlock (lock->fd, flags))
if (!do_repo_unlock (self->lock.fd, flags))
return glnx_throw_errno_prefix (error, "Unlocking repo failed");
}

g_queue_pop_head (&lock->stack);
if (state_to_drop == LOCK_EX)
self->lock.exclusive--;
else
self->lock.shared--;
g_ptr_array_remove_index (stack, stack->len - 1);

return TRUE;
}
Expand Down Expand Up @@ -1065,6 +1090,11 @@ ostree_repo_finalize (GObject *object)
g_private_replace (&repo_lock_table, NULL);
}

g_mutex_lock (&self->lock.mutex);
glnx_close_fd (&self->lock.fd);
g_mutex_unlock (&self->lock.mutex);
g_mutex_clear (&self->lock.mutex);

G_OBJECT_CLASS (ostree_repo_parent_class)->finalize (object);
}

Expand Down Expand Up @@ -1225,6 +1255,7 @@ ostree_repo_init (OstreeRepo *self)
self->test_error_flags = g_parse_debug_string (g_getenv ("OSTREE_REPO_TEST_ERROR"),
test_error_keys, G_N_ELEMENTS (test_error_keys));

g_mutex_init (&self->lock.mutex);
g_mutex_init (&self->cache_lock);
g_mutex_init (&self->txn_lock);

Expand All @@ -1238,6 +1269,7 @@ ostree_repo_init (OstreeRepo *self)
self->tmp_dir_fd = -1;
self->objects_dir_fd = -1;
self->uncompressed_objects_dir_fd = -1;
self->lock.fd = -1;
self->sysroot_kind = OSTREE_REPO_SYSROOT_KIND_UNKNOWN;
}

Expand Down

0 comments on commit b9f0a49

Please sign in to comment.