daemon: Make "diff" methods safer

Various OS "diff" methods can run concurrently with whatever else is
going on since they don't have to obtain the system root lock.

Just to make sure there's no conflicts when writing deployments or
downloading RPM package details, use an internal reader/writer lock
to protect the critical sections of upgrade, rebase, rollback, etc.
This commit is contained in:
Matthew Barnes 2015-09-08 17:41:27 -04:00
parent a22c592a78
commit 73f2a7f058
4 changed files with 148 additions and 16 deletions

View File

@ -176,6 +176,7 @@ get_rebase_diff_variant_in_thread (GTask *task,
GCancellable *cancellable)
{
RPMOSTreeOS *self = RPMOSTREE_OS (object);
RpmostreedSysroot *global_sysroot;
const gchar *name;
glnx_unref_object OstreeSysroot *ot_sysroot = NULL;
@ -188,7 +189,11 @@ get_rebase_diff_variant_in_thread (GTask *task,
GError *error = NULL; /* freed when invoked */
gchar *refspec = data_ptr; /* freed by task */
if (!rpmostreed_sysroot_load_state (rpmostreed_sysroot_get (),
global_sysroot = rpmostreed_sysroot_get ();
rpmostreed_sysroot_reader_lock (global_sysroot);
if (!rpmostreed_sysroot_load_state (global_sysroot,
cancellable,
&ot_sysroot,
&ot_repo,
@ -218,6 +223,8 @@ get_rebase_diff_variant_in_thread (GTask *task,
&error);
out:
rpmostreed_sysroot_reader_unlock (global_sysroot);
set_diff_task_result (task, value, error);
}
@ -228,6 +235,7 @@ get_upgrade_diff_variant_in_thread (GTask *task,
GCancellable *cancellable)
{
RPMOSTreeOS *self = RPMOSTREE_OS (object);
RpmostreedSysroot *global_sysroot;
const gchar *name;
gchar *compare_deployment = data_ptr; /* freed by task */
@ -239,7 +247,11 @@ get_upgrade_diff_variant_in_thread (GTask *task,
GVariant *value = NULL; /* freed when invoked */
GError *error = NULL; /* freed when invoked */
if (!rpmostreed_sysroot_load_state (rpmostreed_sysroot_get (),
global_sysroot = rpmostreed_sysroot_get ();
rpmostreed_sysroot_reader_lock (global_sysroot);
if (!rpmostreed_sysroot_load_state (global_sysroot,
cancellable,
&ot_sysroot,
&ot_repo,
@ -286,6 +298,8 @@ get_upgrade_diff_variant_in_thread (GTask *task,
&error);
out:
rpmostreed_sysroot_reader_unlock (global_sysroot);
set_diff_task_result (task, value, error);
}
@ -295,6 +309,7 @@ get_deployments_diff_variant_in_thread (GTask *task,
gpointer data_ptr,
GCancellable *cancellable)
{
RpmostreedSysroot *global_sysroot;
const gchar *ref0;
const gchar *ref1;
@ -309,7 +324,11 @@ get_deployments_diff_variant_in_thread (GTask *task,
g_return_if_fail (compare_refs->len == 2);
if (!rpmostreed_sysroot_load_state (rpmostreed_sysroot_get (),
global_sysroot = rpmostreed_sysroot_get ();
rpmostreed_sysroot_reader_lock (global_sysroot);
if (!rpmostreed_sysroot_load_state (global_sysroot,
cancellable,
&ot_sysroot,
&ot_repo,
@ -349,6 +368,8 @@ get_deployments_diff_variant_in_thread (GTask *task,
&error);
out:
rpmostreed_sysroot_reader_unlock (global_sysroot);
set_diff_task_result (task, value, error);
}

View File

@ -60,6 +60,13 @@ struct _RpmostreedSysroot {
GHashTable *os_interfaces;
GRWLock children_lock;
/* The OS interface's various diff methods can run concurrently with
* transactions, which is safe except when the transaction is writing
* new deployments to disk or downloading RPM package details. The
* writer lock protects these critical sections so the diff methods
* (holding reader locks) can run safely. */
GRWLock method_rw_lock;
GFileMonitor *monitor;
guint sig_changed;
guint64 last_monitor_event;
@ -580,6 +587,7 @@ sysroot_finalize (GObject *object)
g_hash_table_unref (self->os_interfaces);
g_rw_lock_clear (&self->children_lock);
g_rw_lock_clear (&self->method_rw_lock);
g_clear_object (&self->cancellable);
g_clear_object (&self->monitor);
@ -602,6 +610,7 @@ rpmostreed_sysroot_init (RpmostreedSysroot *self)
(GDestroyNotify) g_object_unref);
g_rw_lock_init (&self->children_lock);
g_rw_lock_init (&self->method_rw_lock);
self->monitor = NULL;
self->sig_changed = 0;
@ -1005,6 +1014,36 @@ rpmostreed_sysroot_get (void)
return _sysroot_instance;
}
void
rpmostreed_sysroot_reader_lock (RpmostreedSysroot *self)
{
g_return_if_fail (RPMOSTREED_IS_SYSROOT (self));
g_rw_lock_reader_lock (&self->method_rw_lock);
}
void
rpmostreed_sysroot_reader_unlock (RpmostreedSysroot *self)
{
g_return_if_fail (RPMOSTREED_IS_SYSROOT (self));
g_rw_lock_reader_unlock (&self->method_rw_lock);
}
void
rpmostreed_sysroot_writer_lock (RpmostreedSysroot *self)
{
g_return_if_fail (RPMOSTREED_IS_SYSROOT (self));
g_rw_lock_writer_lock (&self->method_rw_lock);
}
void
rpmostreed_sysroot_writer_unlock (RpmostreedSysroot *self)
{
g_return_if_fail (RPMOSTREED_IS_SYSROOT (self));
g_rw_lock_writer_unlock (&self->method_rw_lock);
}
/* ---------------------------------------------------------------------------------------------------- */

View File

@ -41,3 +41,7 @@ gboolean rpmostreed_sysroot_load_state (RpmostreedSysroot *self
GError **error);
void rpmostreed_sysroot_emit_update (RpmostreedSysroot *self);
void rpmostreed_sysroot_reader_lock (RpmostreedSysroot *self);
void rpmostreed_sysroot_reader_unlock (RpmostreedSysroot *self);
void rpmostreed_sysroot_writer_lock (RpmostreedSysroot *self);
void rpmostreed_sysroot_writer_unlock (RpmostreedSysroot *self);

View File

@ -24,6 +24,7 @@
#include "rpmostreed-transaction-types.h"
#include "rpmostreed-transaction.h"
#include "rpmostreed-deployment-utils.h"
#include "rpmostreed-sysroot.h"
#include "rpmostreed-utils.h"
static gboolean
@ -77,6 +78,69 @@ out:
return ret;
}
static gboolean
safe_sysroot_upgrader_deploy (OstreeSysrootUpgrader *upgrader,
GCancellable *cancellable,
GError **error)
{
RpmostreedSysroot *global_sysroot = rpmostreed_sysroot_get ();
gboolean success;
rpmostreed_sysroot_writer_lock (global_sysroot);
success = ostree_sysroot_upgrader_deploy (upgrader, cancellable, error);
rpmostreed_sysroot_writer_unlock (global_sysroot);
return success;
}
static gboolean
safe_sysroot_upgrader_pull_package_diff (OstreeSysrootUpgrader *upgrader,
OstreeAsyncProgress *progress,
gboolean *out_changed,
GCancellable *cancellable,
GError **error)
{
RpmostreedSysroot *global_sysroot = rpmostreed_sysroot_get ();
gboolean success;
rpmostreed_sysroot_writer_lock (global_sysroot);
success = ostree_sysroot_upgrader_pull_one_dir (upgrader,
"/usr/share/rpm",
0, 0,
progress,
out_changed,
cancellable,
error);
rpmostreed_sysroot_writer_unlock (global_sysroot);
return success;
}
static gboolean
safe_sysroot_write_deployments (OstreeSysroot *sysroot,
GPtrArray *deployments,
GCancellable *cancellable,
GError **error)
{
RpmostreedSysroot *global_sysroot = rpmostreed_sysroot_get ();
gboolean success;
rpmostreed_sysroot_writer_lock (global_sysroot);
success = ostree_sysroot_write_deployments (sysroot,
deployments,
cancellable,
error);
rpmostreed_sysroot_writer_unlock (global_sysroot);
return success;
}
/* ============================= Package Diff ============================= */
typedef struct {
@ -153,9 +217,9 @@ package_diff_transaction_execute (RpmostreedTransaction *transaction,
progress = ostree_async_progress_new ();
rpmostreed_transaction_connect_download_progress (transaction, progress);
rpmostreed_transaction_connect_signature_progress (transaction, repo);
if (!ostree_sysroot_upgrader_pull_one_dir (upgrader, "/usr/share/rpm",
0, 0, progress, &changed,
cancellable, error))
if (!safe_sysroot_upgrader_pull_package_diff (upgrader, progress, &changed,
cancellable, error))
goto out;
rpmostree_transaction_emit_progress_end (RPMOSTREE_TRANSACTION (transaction));
@ -291,10 +355,13 @@ rollback_transaction_execute (RpmostreedTransaction *transaction,
/* if default changed write it */
if (old_deployments->pdata[0] != new_deployments->pdata[0])
ostree_sysroot_write_deployments (sysroot,
new_deployments,
cancellable,
error);
{
if (!safe_sysroot_write_deployments (sysroot,
new_deployments,
cancellable,
error))
goto out;
}
ret = TRUE;
@ -403,10 +470,11 @@ clear_rollback_transaction_execute (RpmostreedTransaction *transaction,
g_ptr_array_remove_index (deployments, rollback_index);
ostree_sysroot_write_deployments (sysroot,
deployments,
cancellable,
error);
if (!safe_sysroot_write_deployments (sysroot,
deployments,
cancellable,
error))
goto out;
ret = TRUE;
@ -542,7 +610,7 @@ upgrade_transaction_execute (RpmostreedTransaction *transaction,
if (changed)
{
if (!ostree_sysroot_upgrader_deploy (upgrader, cancellable, error))
if (!safe_sysroot_upgrader_deploy (upgrader, cancellable, error))
goto out;
}
else
@ -689,7 +757,7 @@ rebase_transaction_execute (RpmostreedTransaction *transaction,
rpmostree_transaction_emit_progress_end (RPMOSTREE_TRANSACTION (transaction));
if (!ostree_sysroot_upgrader_deploy (upgrader, cancellable, error))
if (!safe_sysroot_upgrader_deploy (upgrader, cancellable, error))
goto out;
if (!self->skip_purge)