lib/repo: Enable locking by default, but drop external API

The code has been sitting around for a while but since I disabled
it by default, I doubt anyone is really using it or relying on it.

This patch and turns on locking by default, and also drops the
API which was only public in the experimental API builds.
Conceptually these are two distinct things, and we
may actually want to split up the patches.

I don't think this will break anyone, but it's hard to say for sure.
It's also going to be hard to find out until we actually release
I suspect...

But anyone who is broken should be able to add `locking=false` into
their repo config.  On the flip side Endless has been shipping with
this enabled and it is reported to help.

The reason to drop the APIs: I'm a bit concerned about the interactions over time
between libostree's use of the API and any apps that start using it.
For example, if an app specifies a SHARED lock in their code, then
later internally we decide to temporarily grab an `EXCLUSIVE`, but the
app had a second thread/process that was `EXCLUSIVE` already, and
that process was waiting on the first bit of code, then we could
deadlock. I can't think of a real world situation where this would happen
yet though.

We are likely to in the future have say `fsck` take an external lock,
`checkout` grab a shared one, etc.

Closes: #1555
Approved by: jlebon
This commit is contained in:
Colin Walters authored and Atomic Bot committed 2018-04-30 17:24:51 +00:00
1 parent 776ce5cc6a
commit 8c1542134c
9 files changed
+48 -108

No files matched your search

+24 -29
View File
@@ -436,7 +436,7 @@ pop_repo_lock (OstreeRepo *self,
return TRUE;
}
/**
/*
* ostree_repo_lock_push:
* @self: a #OstreeRepo
* @lock_type: the type of lock to acquire
@@ -462,13 +462,12 @@ pop_repo_lock (OstreeRepo *self,
* %TRUE is returned.
*
* Returns: %TRUE on success, otherwise %FALSE with @error set
* Since: 2017.14
*/
gboolean
ostree_repo_lock_push (OstreeRepo *self,
OstreeRepoLockType lock_type,
GCancellable *cancellable,
GError **error)
_ostree_repo_lock_push (OstreeRepo *self,
OstreeRepoLockType lock_type,
GCancellable *cancellable,
GError **error)
{
g_return_val_if_fail (self != NULL, FALSE);
g_return_val_if_fail (OSTREE_IS_REPO (self), FALSE);
@@ -531,8 +530,8 @@ ostree_repo_lock_push (OstreeRepo *self,
}
}
/**
* ostree_repo_lock_pop:
/*
* _ostree_repo_lock_pop:
* @self: a #OstreeRepo
* @cancellable: a #GCancellable
* @error: a #GError
@@ -553,12 +552,11 @@ ostree_repo_lock_push (OstreeRepo *self,
* %TRUE is returned.
*
* Returns: %TRUE on success, otherwise %FALSE with @error set
* Since: 2017.14
*/
gboolean
ostree_repo_lock_pop (OstreeRepo *self,
GCancellable *cancellable,
GError **error)
_ostree_repo_lock_pop (OstreeRepo *self,
GCancellable *cancellable,
GError **error)
{
g_return_val_if_fail (self != NULL, FALSE);
g_return_val_if_fail (OSTREE_IS_REPO (self), FALSE);
@@ -621,8 +619,8 @@ ostree_repo_lock_pop (OstreeRepo *self,
}
}
/**
* ostree_repo_auto_lock_push: (skip)
/*
* _ostree_repo_auto_lock_push: (skip)
* @self: a #OstreeRepo
* @lock_type: the type of lock to acquire
* @cancellable: a #GCancellable
@@ -636,37 +634,34 @@ ostree_repo_lock_pop (OstreeRepo *self,
*
* |[<!-- language="C" -->
* g_autoptr(OstreeRepoAutoLock) lock = NULL;
* lock = ostree_repo_auto_lock_push (repo, lock_type, cancellable, error);
* lock = _ostree_repo_auto_lock_push (repo, lock_type, cancellable, error);
* if (!lock)
* return FALSE;
* ]|
*
* Returns: @self on success, otherwise %NULL with @error set
* Since: 2017.14
*/
OstreeRepoAutoLock *
ostree_repo_auto_lock_push (OstreeRepo *self,
OstreeRepoLockType lock_type,
GCancellable *cancellable,
GError **error)
_ostree_repo_auto_lock_push (OstreeRepo *self,
OstreeRepoLockType lock_type,
GCancellable *cancellable,
GError **error)
{
if (!ostree_repo_lock_push (self, lock_type, cancellable, error))
if (!_ostree_repo_lock_push (self, lock_type, cancellable, error))
return NULL;
return (OstreeRepoAutoLock *)self;
}
/**
* ostree_repo_auto_lock_cleanup: (skip)
/*
* _ostree_repo_auto_lock_cleanup: (skip)
* @lock: a #OstreeRepoAutoLock
*
* A cleanup handler for use with ostree_repo_auto_lock_push(). If @lock is
* not %NULL, ostree_repo_lock_pop() will be called on it. If
* ostree_repo_lock_pop() fails, a critical warning will be emitted.
*
* Since: 2017.14
*/
void
ostree_repo_auto_lock_cleanup (OstreeRepoAutoLock *lock)
_ostree_repo_auto_lock_cleanup (OstreeRepoAutoLock *lock)
{
OstreeRepo *repo = lock;
if (repo)
@@ -674,7 +669,7 @@ ostree_repo_auto_lock_cleanup (OstreeRepoAutoLock *lock)
g_autoptr(GError) error = NULL;
int errsv = errno;
if (!ostree_repo_lock_pop (repo, NULL, &error))
if (!_ostree_repo_lock_pop (repo, NULL, &error))
g_critical ("Cleanup repo lock failed: %s", error->message);
errno = errsv;
@@ -2742,10 +2737,10 @@ reload_core_config (OstreeRepo *self,
self->tmp_expiry_seconds = g_ascii_strtoull (tmp_expiry_seconds, NULL, 10);
}
/* Disable locking by default for now */
{ gboolean locking;
/* Enabled by default in 2018.05 */
if (!ot_keyfile_get_boolean_with_default (self->config, "core", "locking",
FALSE, &locking, error))
TRUE, &locking, error))
return FALSE;
if (!locking)
{