diff --git a/src/daemon/rpmostreed-os.c b/src/daemon/rpmostreed-os.c index 769e8a93..9be63468 100644 --- a/src/daemon/rpmostreed-os.c +++ b/src/daemon/rpmostreed-os.c @@ -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); } diff --git a/src/daemon/rpmostreed-sysroot.c b/src/daemon/rpmostreed-sysroot.c index ac267023..51cb7818 100644 --- a/src/daemon/rpmostreed-sysroot.c +++ b/src/daemon/rpmostreed-sysroot.c @@ -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); +} /* ---------------------------------------------------------------------------------------------------- */ diff --git a/src/daemon/rpmostreed-sysroot.h b/src/daemon/rpmostreed-sysroot.h index 1d88df81..a3358494 100644 --- a/src/daemon/rpmostreed-sysroot.h +++ b/src/daemon/rpmostreed-sysroot.h @@ -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); diff --git a/src/daemon/rpmostreed-transaction-types.c b/src/daemon/rpmostreed-transaction-types.c index e1ef4621..adce7a9a 100644 --- a/src/daemon/rpmostreed-transaction-types.c +++ b/src/daemon/rpmostreed-transaction-types.c @@ -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)