checkpoint: let each checkpoint schedule its own timeout

Instead of having NMCheckpointManager schedule one timeout
for all, let each NMCheckpoint do it itself.

3 files changed, 65 insertions, 66 deletions

diff --git a/src/nm-checkpoint-manager.c b/src/nm-checkpoint-manager.c
index c200545c82..a88c4a763c 100644
--- a/src/nm-checkpoint-manager.c
+++ b/src/nm-checkpoint-manager.c
@@ -36,7 +36,6 @@ struct _NMCheckpointManager {
 	NMManager *_manager;
 	GParamSpec *property_spec;
 	CList checkpoints_lst_head;
-	guint rollback_timeout_id;
 };
 
 #define GET_MANAGER(self) \
@@ -57,10 +56,6 @@ struct _NMCheckpointManager {
 
 /*****************************************************************************/
 
-static void update_rollback_timeout (NMCheckpointManager *self);
-
-/*****************************************************************************/
-
 static void
 notify_checkpoints (NMCheckpointManager *self) {
 	g_object_notify_by_pspec ((GObject *) GET_MANAGER (self),
@@ -74,6 +69,8 @@ destroy_checkpoint (NMCheckpointManager *self, NMCheckpoint *checkpoint)
 	nm_assert (nm_dbus_object_is_exported (NM_DBUS_OBJECT (checkpoint)));
 	nm_assert (c_list_contains (&self->checkpoints_lst_head, &checkpoint->checkpoints_lst));
 
+	nm_checkpoint_set_timeout_callback (checkpoint, NULL, NULL);
+
 	c_list_unlink (&checkpoint->checkpoints_lst);
 
 	notify_checkpoints (self);
@@ -92,51 +89,14 @@ rollback_checkpoint (NMCheckpointManager *self, NMCheckpoint *checkpoint)
 	return result;
 }
 
-static gboolean
-rollback_timeout_cb (NMCheckpointManager *self)
-{
-	NMCheckpoint *checkpoint, *checkpoint_safe;
-	gint64 ts, now;
-
-	self->rollback_timeout_id = 0;
-
-	now = nm_utils_get_monotonic_timestamp_ms ();
-
-	c_list_for_each_entry_safe (checkpoint, checkpoint_safe, &self->checkpoints_lst_head, checkpoints_lst) {
-		ts = nm_checkpoint_get_rollback_ts (checkpoint);
-		if (ts && ts <= now) {
-			gs_unref_variant GVariant *result = NULL;
-
-			result = rollback_checkpoint (self, checkpoint);
-		}
-	}
-
-	update_rollback_timeout (self);
-
-	return G_SOURCE_REMOVE;
-}
-
 static void
-update_rollback_timeout (NMCheckpointManager *self)
+rollback_timeout_cb (NMCheckpoint *checkpoint,
+                     gpointer user_data)
 {
-	NMCheckpoint *checkpoint;
-	gint64 ts, delta, next = G_MAXINT64;
+	NMCheckpointManager *self = user_data;
+	gs_unref_variant GVariant *result = NULL;
 
-	c_list_for_each_entry (checkpoint, &self->checkpoints_lst_head, checkpoints_lst) {
-		ts = nm_checkpoint_get_rollback_ts (checkpoint);
-		if (ts && ts < next)
-			next = ts;
-	}
-
-	nm_clear_g_source (&self->rollback_timeout_id);
-
-	if (next != G_MAXINT64) {
-		delta = MAX (next - nm_utils_get_monotonic_timestamp_ms (), 0);
-		self->rollback_timeout_id = g_timeout_add (delta,
-		                                           (GSourceFunc) rollback_timeout_cb,
-		                                           self);
-		_LOGT ("update timeout: next check in %" G_GINT64_FORMAT " ms", delta);
-	}
+	result = rollback_checkpoint (self, checkpoint);
 }
 
 static NMCheckpoint *
@@ -224,9 +184,9 @@ nm_checkpoint_manager_create (NMCheckpointManager *self,
 
 	nm_dbus_object_export (NM_DBUS_OBJECT (checkpoint));
 
+	nm_checkpoint_set_timeout_callback (checkpoint, rollback_timeout_cb, self);
 	c_list_link_tail (&self->checkpoints_lst_head, &checkpoint->checkpoints_lst);
 	notify_checkpoints (self);
-	update_rollback_timeout (self);
 	return checkpoint;
 }
 
@@ -361,6 +321,5 @@ nm_checkpoint_manager_free (NMCheckpointManager *self)
 		return;
 
 	nm_checkpoint_manager_destroy_all (self);
-	nm_clear_g_source (&self->rollback_timeout_id);
 	g_slice_free (NMCheckpointManager, self);
 }
diff --git a/src/nm-checkpoint.c b/src/nm-checkpoint.c
index a202c0639f..4975e6ec69 100644
--- a/src/nm-checkpoint.c
+++ b/src/nm-checkpoint.c
@@ -57,13 +57,17 @@ NM_GOBJECT_PROPERTIES_DEFINE_BASE (
 struct _NMCheckpointPrivate {
 	/* properties */
 	GHashTable *devices;
-	gint64 created;
-	guint32 rollback_timeout;
+	gint64 created_at_ms;
+	guint32 rollback_timeout_s;
+	guint timeout_id;
+	/* private members */
 	/* private members */
 	NMManager *manager;
-	gint64 rollback_ts;
 	NMCheckpointCreateFlags flags;
 	GHashTable *connection_uuids;
+
+	NMCheckpointTimeoutCallback timeout_cb;
+	gpointer timeout_data;
 };
 
 struct _NMCheckpointClass {
@@ -96,12 +100,18 @@ G_DEFINE_TYPE (NMCheckpoint, nm_checkpoint, NM_TYPE_DBUS_OBJECT)
 
 /*****************************************************************************/
 
-guint64
-nm_checkpoint_get_rollback_ts (NMCheckpoint *self)
+void
+nm_checkpoint_set_timeout_callback (NMCheckpoint *self,
+                                    NMCheckpointTimeoutCallback callback,
+                                    gpointer user_data)
 {
-	g_return_val_if_fail (NM_IS_CHECKPOINT (self), 0);
+	NMCheckpointPrivate *priv = NM_CHECKPOINT_GET_PRIVATE (self);
 
-	return NM_CHECKPOINT_GET_PRIVATE (self)->rollback_ts;
+	/* in glib world, we would have a GSignal for this. But as there
+	 * is only one subscriber, it's simpler to just set and unset(!)
+	 * the callback this was. */
+	priv->timeout_cb = callback;
+	priv->timeout_data = user_data;
 }
 
 gboolean
@@ -374,6 +384,7 @@ device_checkpoint_create (NMDevice *device)
 	NMActRequest *act_request;
 
 	nm_assert (NM_IS_DEVICE (device));
+	nm_assert (nm_device_is_real (device));
 
 	path = nm_dbus_object_get_path (NM_DBUS_OBJECT (device));
 
@@ -421,6 +432,21 @@ device_checkpoint_destroy (gpointer data)
 	g_slice_free (DeviceCheckpoint, dev_checkpoint);
 }
 
+static gboolean
+_timeout_cb (gpointer user_data)
+{
+	NMCheckpoint *self = user_data;
+	NMCheckpointPrivate *priv = NM_CHECKPOINT_GET_PRIVATE (self);
+
+	priv->timeout_id = 0;
+
+	if (priv->timeout_cb)
+		priv->timeout_cb (self, priv->timeout_data);
+
+	/* beware, @self likely got destroyed! */
+	return G_SOURCE_REMOVE;
+}
+
 /*****************************************************************************/
 
 static void
@@ -437,10 +463,12 @@ get_property (GObject *object, guint prop_id,
 		                                                 FALSE);
 		break;
 	case PROP_CREATED:
-		g_value_set_int64 (value, priv->created);
+		g_value_set_int64 (value,
+		                   nm_utils_monotonic_timestamp_as_boottime (priv->created_at_ms,
+		                                                             NM_UTILS_NS_PER_MSEC));
 		break;
 	case PROP_ROLLBACK_TIMEOUT:
-		g_value_set_uint (value, priv->rollback_timeout);
+		g_value_set_uint (value, priv->rollback_timeout_s);
 		break;
 	default:
 		G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec);
@@ -466,12 +494,13 @@ nm_checkpoint_init (NMCheckpoint *self)
 }
 
 NMCheckpoint *
-nm_checkpoint_new (NMManager *manager, GPtrArray *devices, guint32 rollback_timeout,
+nm_checkpoint_new (NMManager *manager, GPtrArray *devices, guint32 rollback_timeout_s,
                    NMCheckpointCreateFlags flags, GError **error)
 {
 	NMCheckpoint *self;
 	NMCheckpointPrivate *priv;
 	NMSettingsConnection *const *con;
+	gint64 rollback_timeout_ms;
 	guint i;
 
 	g_return_val_if_fail (manager, NULL);
@@ -490,14 +519,17 @@ nm_checkpoint_new (NMManager *manager, GPtrArray *devices, guint32 rollback_time
 
 	priv = NM_CHECKPOINT_GET_PRIVATE (self);
 	priv->manager = manager;
-	priv->created = nm_utils_monotonic_timestamp_as_boottime (nm_utils_get_monotonic_timestamp_ms (),
-	                                                          NM_UTILS_NS_PER_MSEC);
-	priv->rollback_timeout = rollback_timeout;
-	priv->rollback_ts = rollback_timeout ?
-	    (nm_utils_get_monotonic_timestamp_ms () + ((gint64) rollback_timeout * 1000)) :
-	    0;
+	priv->rollback_timeout_s = rollback_timeout_s;
+	priv->created_at_ms = nm_utils_get_monotonic_timestamp_ms ();
 	priv->flags = flags;
 
+	if (rollback_timeout_s != 0) {
+		rollback_timeout_ms = ((gint64) rollback_timeout_s) * 1000;
+		priv->timeout_id = g_timeout_add (NM_MIN (rollback_timeout_ms, (gint64) G_MAXUINT32),
+		                                  _timeout_cb,
+		                                  self);
+	}
+
 	if (NM_FLAGS_HAS (flags, NM_CHECKPOINT_CREATE_FLAG_DELETE_NEW_CONNECTIONS)) {
 		priv->connection_uuids = g_hash_table_new_full (nm_str_hash, g_str_equal, g_free, NULL);
 		for (con = nm_settings_get_connections (nm_settings_get (), NULL); *con; con++) {
@@ -528,6 +560,8 @@ dispose (GObject *object)
 	g_clear_pointer (&priv->devices, g_hash_table_unref);
 	g_clear_pointer (&priv->connection_uuids, g_hash_table_unref);
 
+	nm_clear_g_source (&priv->timeout_id);
+
 	G_OBJECT_CLASS (nm_checkpoint_parent_class)->dispose (object);
 }
 
diff --git a/src/nm-checkpoint.h b/src/nm-checkpoint.h
index 186c2f3803..59490322e3 100644
--- a/src/nm-checkpoint.h
+++ b/src/nm-checkpoint.h
@@ -50,7 +50,13 @@ GType nm_checkpoint_get_type (void);
 NMCheckpoint *nm_checkpoint_new (NMManager *manager, GPtrArray *devices, guint32 rollback_timeout,
                                  NMCheckpointCreateFlags flags, GError **error);
 
-guint64 nm_checkpoint_get_rollback_ts (NMCheckpoint *checkpoint);
+typedef void (*NMCheckpointTimeoutCallback) (NMCheckpoint *self,
+                                             gpointer user_data);
+
+void nm_checkpoint_set_timeout_callback (NMCheckpoint *self,
+                                         NMCheckpointTimeoutCallback callback,
+                                         gpointer user_data);
+
 gboolean nm_checkpoint_includes_device (NMCheckpoint *checkpoint, NMDevice *device);
 GVariant *nm_checkpoint_rollback (NMCheckpoint *self);