libnm/secret-agent: merge branch 'th/secret-agent-rework'

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/merge_requests/383

libnm-core/tests/test-general.c

@@ -8270,7 +8270,7 @@ test_integrate_maincontext (gconstpointer test_data)
 		g_source_set_callback (idle_source_1, _test_integrate_maincontext_cb_idle1, &count, NULL);
 		g_source_attach (idle_source_1, c2);
 
-		nmtst_main_context_iterate_until (c1, 2000, count == 5);
+		nmtst_main_context_iterate_until_assert (c1, 2000, count == 5);
 	}
 
 	if (TEST_IDX == 2) {

libnm/libnm.ver

@@ -1667,6 +1667,12 @@ global:
 	nm_device_vrf_get_table;
 	nm_device_vrf_get_type;
 	nm_object_get_client;
+	nm_secret_agent_old_destroy;
+	nm_secret_agent_old_enable;
+	nm_secret_agent_old_get_context_busy_watcher;
+	nm_secret_agent_old_get_dbus_connection;
+	nm_secret_agent_old_get_dbus_name_owner;
+	nm_secret_agent_old_get_main_context;
 	nm_setting_vrf_get_table;
 	nm_setting_vrf_get_type;
 	nm_setting_vrf_new;

libnm/nm-client.c

@@ -60,7 +60,7 @@
 
 /*****************************************************************************/
 
-static NM_CACHED_QUARK_FCN ("nm-client-context-busy-watcher", nm_client_context_busy_watcher_quark)
+NM_CACHED_QUARK_FCN ("nm-context-busy-watcher", nm_context_busy_watcher_quark)
 
 static void
 _context_busy_watcher_attach_integration_source_cb (gpointer data,
@@ -69,10 +69,21 @@ _context_busy_watcher_attach_integration_source_cb (gpointer data,
 	nm_g_source_destroy_and_unref (data);
 }
 
-static void
+void
-_context_busy_watcher_attach_integration_source (GObject *context_busy_watcher,
+nm_context_busy_watcher_integrate_source (GMainContext *outer_context,
-                                                 GSource *source_take)
+                                          GMainContext *inner_context,
+                                          GObject *context_busy_watcher)
 {
+	GSource *source;
+
+	nm_assert (outer_context);
+	nm_assert (inner_context);
+	nm_assert (outer_context != inner_context);
+	nm_assert (G_IS_OBJECT (context_busy_watcher));
+
+	source = nm_utils_g_main_context_create_integrate_source (inner_context);
+	g_source_attach (source, outer_context);
+
 	/* The problem is...
 	 *
 	 * NMClient is associated with a GMainContext, just like its underlying GDBusConnection
@@ -114,7 +125,7 @@ _context_busy_watcher_attach_integration_source (GObject *context_busy_watcher,
 
 	g_object_weak_ref (context_busy_watcher,
 	                   _context_busy_watcher_attach_integration_source_cb,
-	                   source_take);
+	                   source);
 }
 
 /*****************************************************************************/
@@ -175,22 +186,6 @@ typedef struct {
 
 /*****************************************************************************/
 
-typedef struct {
-	GCancellable *cancellable;
-	GSource *cancel_on_idle_source;
-	gulong cancelled_id;
-	union {
-		struct {
-			GTask *task;
-		} async;
-		struct {
-			GMainLoop *main_loop;
-			GError **error_location;
-		} sync;
-	} data;
-	bool is_sync:1;
-} InitData;
-
 NM_GOBJECT_PROPERTIES_DEFINE (NMClient,
 	PROP_DBUS_CONNECTION,
 	PROP_DBUS_NAME_OWNER,
@@ -263,7 +258,7 @@ typedef struct {
 	GMainContext *dbus_context;
 	GObject *context_busy_watcher;
 	GDBusConnection *dbus_connection;
-	InitData *init_data;
+	NMLInitData *init_data;
 	GHashTable *dbus_objects;
 	CList obj_changed_lst_head;
 	GCancellable *name_owner_get_cancellable;
@@ -409,15 +404,15 @@ NM_CACHED_QUARK_FCN ("nm-client-error-quark", nm_client_error_quark)
 
 /*****************************************************************************/
 
-static InitData *
+NMLInitData *
-_init_data_new_sync (GCancellable *cancellable,
+nml_init_data_new_sync (GCancellable *cancellable,
-                     GMainLoop *main_loop,
+                        GMainLoop *main_loop,
-                     GError **error_location)
+                        GError **error_location)
 {
-	InitData *init_data;
+	NMLInitData *init_data;
 
-	init_data = g_slice_new (InitData);
+	init_data = g_slice_new (NMLInitData);
-	*init_data = (InitData) {
+	*init_data = (NMLInitData) {
 		.cancellable = nm_g_object_ref (cancellable),
 		.is_sync     = TRUE,
 		.data.sync   = {
@@ -428,14 +423,14 @@ _init_data_new_sync (GCancellable *cancellable,
 	return init_data;
 }
 
-static InitData *
+NMLInitData *
-_init_data_new_async (GCancellable *cancellable,
+nml_init_data_new_async (GCancellable *cancellable,
-                      GTask *task_take)
+                         GTask *task_take)
 {
-	InitData *init_data;
+	NMLInitData *init_data;
 
-	init_data = g_slice_new (InitData);
+	init_data = g_slice_new (NMLInitData);
-	*init_data = (InitData) {
+	*init_data = (NMLInitData) {
 		.cancellable = nm_g_object_ref (cancellable),
 		.is_sync     = FALSE,
 		.data.async  = {
@@ -445,6 +440,30 @@ _init_data_new_async (GCancellable *cancellable,
 	return init_data;
 }
 
+void
+nml_init_data_return (NMLInitData *init_data,
+                      GError *error_take)
+{
+	nm_assert (init_data);
+
+	nm_clear_pointer (&init_data->cancel_on_idle_source, nm_g_source_destroy_and_unref);
+	nm_clear_g_signal_handler (init_data->cancellable, &init_data->cancelled_id);
+
+	if (init_data->is_sync) {
+		if (error_take)
+			g_propagate_error (init_data->data.sync.error_location, error_take);
+		g_main_loop_quit (init_data->data.sync.main_loop);
+	} else {
+		if (error_take)
+			g_task_return_error (init_data->data.async.task, error_take);
+		else
+			g_task_return_boolean (init_data->data.async.task, TRUE);
+		g_object_unref (init_data->data.async.task);
+	}
+	nm_g_object_unref (init_data->cancellable);
+	nm_g_slice_free (init_data);
+}
+
 /*****************************************************************************/
 
 GError *
@@ -1008,7 +1027,7 @@ nm_client_get_context_busy_watcher (NMClient *self)
 	g_return_val_if_fail (NM_IS_CLIENT (self), NULL);
 
 	w = NM_CLIENT_GET_PRIVATE (self)->context_busy_watcher;
-	return    g_object_get_qdata (w, nm_client_context_busy_watcher_quark ())
+	return    g_object_get_qdata (w, nm_context_busy_watcher_quark ())
 	       ?: w;
 }
 
@@ -6816,7 +6835,7 @@ name_owner_changed (NMClient *self,
 
 		old_context_busy_watcher = g_steal_pointer (&priv->context_busy_watcher);
 		priv->context_busy_watcher = g_object_ref (g_object_get_qdata (old_context_busy_watcher,
-		                                                               nm_client_context_busy_watcher_quark ()));
+		                                                               nm_context_busy_watcher_quark ()));
 
 		g_main_context_ref (priv->main_context);
 		g_main_context_unref (priv->dbus_context);
@@ -6952,35 +6971,80 @@ name_owner_get_call (NMClient *self)
 
 /*****************************************************************************/
 
+static inline gboolean
+_nml_cleanup_context_busy_watcher_on_idle_cb (gpointer user_data)
+{
+	nm_auto_unref_gmaincontext GMainContext *context = NULL;
+	gs_unref_object GObject *context_busy_watcher = NULL;
+
+	nm_utils_user_data_unpack (user_data, &context, &context_busy_watcher);
+
+	nm_assert (context);
+	nm_assert (G_IS_OBJECT (context_busy_watcher));
+	return G_SOURCE_REMOVE;
+}
+
+void
+nml_cleanup_context_busy_watcher_on_idle (GObject *context_busy_watcher_take,
+                                          GMainContext *context)
+{
+	gs_unref_object GObject *context_busy_watcher = g_steal_pointer (&context_busy_watcher_take);
+	GSource *cleanup_source;
+
+	nm_assert (G_IS_OBJECT (context_busy_watcher));
+	nm_assert (context);
+
+	/* Technically, we cancelled all pending actions (and these actions
+	 * (GTask) keep the context_busy_watcher object alive). Also, we passed
+	 * no destroy notify to g_dbus_connection_signal_subscribe().
+	 * That means, there should be no other unaccounted GSource'es left.
+	 *
+	 * However, we really need to be sure that the context_busy_watcher's
+	 * lifetime matches the time that the context is busy. That is especially
+	 * important with synchronous initialization, where the context-busy-watcher
+	 * keeps the inner GMainContext integrated in the caller's.
+	 * We must not g_source_destroy() that integration too early.
+	 *
+	 * So to be really sure all this is given, always schedule one last
+	 * cleanup idle action with low priority. This should be the last
+	 * thing related to this instance that keeps the context busy.
+	 *
+	 * Note that we could also *not* take a reference on @context
+	 * and unref @context_busy_watcher via the GDestroyNotify. That would
+	 * allow for the context to be wrapped up early, and when the last user
+	 * gives up the reference to the context, the destroy notify could complete
+	 * without even invoke the idle handler. However, that destroy notify may
+	 * not be called in the right thread. So, we want to be sure that we unref
+	 * the context-busy-watcher in the right context. Hence, we always take an
+	 * additional reference and always cleanup in the idle handler. This means:
+	 * the user *MUST* always keep iterating the context after NMClient got destroyed.
+	 * But that is not a severe limitation, because the user anyway must be prepared
+	 * to do that. That is because in many cases it is necessary anyway (and the user
+	 * wouldn't know a priory when not). This way, it is just always necessary. */
+
+	cleanup_source = nm_g_idle_source_new (G_PRIORITY_LOW + 10,
+	                                       _nml_cleanup_context_busy_watcher_on_idle_cb,
+	                                       nm_utils_user_data_pack (g_main_context_ref (context),
+	                                                                g_steal_pointer (&context_busy_watcher)),
+	                                       NULL);
+	g_source_attach (cleanup_source, context);
+	g_source_unref (cleanup_source);
+}
+
+/*****************************************************************************/
+
 static void
 _init_start_complete (NMClient *self,
                       GError *error_take)
 {
 	NMClientPrivate *priv = NM_CLIENT_GET_PRIVATE (self);
-	InitData *init_data;
-
-	init_data = g_steal_pointer (&priv->init_data);
 
 	NML_NMCLIENT_LOG_D (self, "%s init complete with %s%s%s",
-	                   init_data->is_sync ? "sync" : "async",
+	                   priv->init_data->is_sync ? "sync" : "async",
 	                   NM_PRINT_FMT_QUOTED (error_take, "error: ", error_take->message, "", "success"));
 
-	nm_clear_pointer (&init_data->cancel_on_idle_source, nm_g_source_destroy_and_unref);
+	nml_init_data_return (g_steal_pointer (&priv->init_data),
-	nm_clear_g_signal_handler (init_data->cancellable, &init_data->cancelled_id);
+	                      error_take);
-
-	if (init_data->is_sync) {
-		if (error_take)
-			g_propagate_error (init_data->data.sync.error_location, error_take);
-		g_main_loop_quit (init_data->data.sync.main_loop);
-	} else {
-		if (error_take)
-			g_task_return_error (init_data->data.async.task, error_take);
-		else
-			g_task_return_boolean (init_data->data.async.task, TRUE);
-		g_object_unref (init_data->data.async.task);
-	}
-	nm_g_object_unref (init_data->cancellable);
-	nm_g_slice_free (init_data);
 }
 
 static void
@@ -7379,12 +7443,12 @@ init_sync (GInitable *initable, GCancellable *cancellable, GError **error)
 	 * to resync and drop the inner context. That means, requests made against the inner
 	 * context have a different lifetime. Hence, we create a separate tracking
 	 * object. This "wraps" the outer context-busy-watcher and references it, so
-	 * that the work together. Grep for nm_client_context_busy_watcher_quark() to
+	 * that the work together. Grep for nm_context_busy_watcher_quark() to
 	 * see how this works. */
 	parent_context_busy_watcher = g_steal_pointer (&priv->context_busy_watcher);
 	priv->context_busy_watcher = g_object_new (G_TYPE_OBJECT, NULL);
 	g_object_set_qdata_full (priv->context_busy_watcher,
-	                         nm_client_context_busy_watcher_quark (),
+	                         nm_context_busy_watcher_quark (),
 	                         parent_context_busy_watcher,
 	                         g_object_unref);
 
@@ -7392,7 +7456,7 @@ init_sync (GInitable *initable, GCancellable *cancellable, GError **error)
 
 	main_loop = g_main_loop_new (dbus_context, FALSE);
 
-	priv->init_data = _init_data_new_sync (cancellable, main_loop, &local_error);
+	priv->init_data = nml_init_data_new_sync (cancellable, main_loop, &local_error);
 
 	_init_start (self);
 
@@ -7403,12 +7467,9 @@ init_sync (GInitable *initable, GCancellable *cancellable, GError **error)
 	g_main_context_pop_thread_default (dbus_context);
 
 	if (priv->main_context != priv->dbus_context) {
-		GSource *source;
+		nm_context_busy_watcher_integrate_source (priv->main_context,
-
+		                                          priv->dbus_context,
-		source = nm_utils_g_main_context_create_integrate_source (priv->dbus_context);
+		                                          priv->context_busy_watcher);
-		g_source_attach (source, priv->main_context);
-		_context_busy_watcher_attach_integration_source (priv->context_busy_watcher,
-		                                                 g_steal_pointer (&source));
 	}
 
 	g_main_context_unref (dbus_context);
@@ -7448,7 +7509,7 @@ init_async (GAsyncInitable *initable,
 	task = nm_g_task_new (self, cancellable, init_async, callback, user_data);
 	g_task_set_priority (task, io_priority);
 
-	priv->init_data = _init_data_new_async (cancellable, g_steal_pointer (&task));
+	priv->init_data = nml_init_data_new_async (cancellable, g_steal_pointer (&task));
 
 	_init_start (self);
 }
@@ -7569,18 +7630,6 @@ constructed (GObject *object)
 	NML_NMCLIENT_LOG_D (self, "new NMClient instance");
 }
 
-static inline gboolean
-_dispose_cleanup_context_busy_watcher_cb (gpointer user_data)
-{
-	nm_auto_unref_gmaincontext GMainContext *context = NULL;
-	gs_unref_object GObject *context_busy_watcher = NULL;
-
-	nm_utils_user_data_unpack (user_data, &context, &context_busy_watcher);
-
-	nm_assert (G_IS_OBJECT (context_busy_watcher));
-	return G_SOURCE_REMOVE;
-}
-
 static void
 dispose (GObject *object)
 {
@@ -7631,42 +7680,8 @@ dispose (GObject *object)
 
 	if (   priv->context_busy_watcher
 	    && priv->dbus_context) {
-		GSource *cleanup_source;
+		nml_cleanup_context_busy_watcher_on_idle (g_steal_pointer (&priv->context_busy_watcher),
-
+		                                          priv->dbus_context);
-		/* Technically, we cancelled all pending actions (and these actions
-		 * (GTask) keep the context_busy_watcher object alive). Also, we passed
-		 * no destroy notify to g_dbus_connection_signal_subscribe().
-		 * That means, there should be no other unaccounted GSource'es left.
-		 *
-		 * However, we really need to be sure that the context_busy_watcher's
-		 * lifetime matches the time that the context is busy. That is especially
-		 * important with synchronous initialization, where the context-busy-watcher
-		 * keeps the inner GMainContext integrated in the caller's.
-		 * We must not g_source_destroy() that integration too early.
-		 *
-		 * So to be really sure all this is given, always schedule one last
-		 * cleanup idle action with low priority. This should be the last
-		 * thing related to this instance that keeps the context busy.
-		 *
-		 * Note that we could also *not* take a reference on priv->dbus_context
-		 * and unref priv->context_busy_watcher via the GDestroyNotify. That would
-		 * allow for the context to be wrapped up early, and when the last user
-		 * gives up the reference to the context, the destroy notify could complete
-		 * without even invoke the idle handler. However, that destroy notify may
-		 * not be called in the right thread. So, we want to be sure that we unref
-		 * the context-busy-watcher in the right context. Hence, we always take an
-		 * additional reference and always cleanup in the idle handler. This means:
-		 * the user *MUST* always keep iterating the context after NMClient got destroyed.
-		 * But that is not a severe limitation, because the user anyway must be prepared
-		 * to do that. That is because in many cases it is necessary anyway (and the user
-		 * wouldn't know a priory when not). This way, it is just always necessary. */
-		cleanup_source = nm_g_idle_source_new (G_PRIORITY_LOW + 10,
-		                                       _dispose_cleanup_context_busy_watcher_cb,
-		                                       nm_utils_user_data_pack (g_main_context_ref (priv->dbus_context),
-		                                                                g_steal_pointer (&priv->context_busy_watcher)),
-		                                       NULL);
-		g_source_attach (cleanup_source, priv->dbus_context);
-		g_source_unref (cleanup_source);
 	}
 
 	nm_clear_pointer (&priv->dbus_context, g_main_context_unref);
@@ -7682,6 +7697,8 @@ dispose (GObject *object)
 
 	priv->nm.capabilities_len = 0;
 	nm_clear_g_free (&priv->nm.capabilities_arr);
+
+	NML_NMCLIENT_LOG_D (self, "disposed");
 }
 
 const NMLDBusMetaIface _nml_dbus_meta_iface_nm_agentmanager = NML_DBUS_META_IFACE_INIT (

libnm/nm-device.c

@@ -158,7 +158,7 @@ _notify_event_state_changed (NMClient *client,
 	gs_unref_object NMDevice *self = notify_event->user_data;
 	NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self);
 
-	NML_NMCLIENT_LOG_T (_nm_object_get_client (self),
+	NML_NMCLIENT_LOG_T (client,
 	                    "[%s] emit Device's StateChanged signal %u -> %u, reason: %u",
 	                    _nm_object_get_path (self),
 	                    (guint) priv->old_state,

libnm/nm-libnm-utils.h

@@ -31,6 +31,8 @@ gboolean nm_utils_g_param_spec_is_default (const GParamSpec *pspec);
 /*****************************************************************************/
 
 typedef enum {
+	_NML_DBUS_LOG_LEVEL_NONE        = 0x00,
+
 	_NML_DBUS_LOG_LEVEL_INITIALIZED = 0x01,
 
 	_NML_DBUS_LOG_LEVEL_TRACE       = 0x02,
@@ -83,6 +85,7 @@ nml_dbus_log_enabled (NMLDBusLogLevel level)
 	int l;
 
 	nm_assert (NM_IN_SET (level, NML_DBUS_LOG_LEVEL_ANY,
+	                             _NML_DBUS_LOG_LEVEL_NONE,
 	                             NML_DBUS_LOG_LEVEL_TRACE,
 	                             NML_DBUS_LOG_LEVEL_DEBUG,
 	                             NML_DBUS_LOG_LEVEL_WARN,
@@ -104,7 +107,8 @@ void _nml_dbus_log (NMLDBusLogLevel level,
 
 #define NML_DBUS_LOG(level, ...) \
 	G_STMT_START { \
-		G_STATIC_ASSERT (   (level) == NML_DBUS_LOG_LEVEL_TRACE \
+		G_STATIC_ASSERT (   (level) == _NML_DBUS_LOG_LEVEL_NONE \
+		                 || (level) == NML_DBUS_LOG_LEVEL_TRACE \
 		                 || (level) == NML_DBUS_LOG_LEVEL_DEBUG \
 		                 || (level) == NML_DBUS_LOG_LEVEL_WARN \
 		                 || (level) == NML_DBUS_LOG_LEVEL_ERROR); \
@@ -119,8 +123,19 @@ void _nml_dbus_log (NMLDBusLogLevel level,
 #define NML_DBUS_LOG_W(...) NML_DBUS_LOG (NML_DBUS_LOG_LEVEL_WARN,  __VA_ARGS__)
 #define NML_DBUS_LOG_E(...) NML_DBUS_LOG (NML_DBUS_LOG_LEVEL_ERROR, __VA_ARGS__)
 
+/* _NML_NMCLIENT_LOG_LEVEL_COERCE is only for printf debugging. You can disable client logging by
+ * mapping the requested log level to a different one (or disable it altogether).
+ * That's useful for example if you are interested in *other* trace logging messages from
+ * libnm and don't want to get flooded by NMClient's trace messages. */
+#define _NML_NMCLIENT_LOG_LEVEL_COERCE(level) \
+	/* for example, change condition below to suppress <trace> messages from NMClient. */ \
+	((   TRUE \
+	  || ((level) != NML_DBUS_LOG_LEVEL_TRACE)) \
+	 ? (level) \
+	 : _NML_DBUS_LOG_LEVEL_NONE)
+
 #define NML_NMCLIENT_LOG(level, self, ...) \
-	NML_DBUS_LOG ((level), \
+	NML_DBUS_LOG (_NML_NMCLIENT_LOG_LEVEL_COERCE (level), \
 	              "nmclient["NM_HASH_OBFUSCATE_PTR_FMT"]: " _NM_UTILS_MACRO_FIRST (__VA_ARGS__), \
 	              NM_HASH_OBFUSCATE_PTR (self) \
 	              _NM_UTILS_MACRO_REST (__VA_ARGS__))
@@ -160,6 +175,45 @@ _nml_coerce_property_strv_not_null (char **strv)
 
 /*****************************************************************************/
 
+GQuark nm_context_busy_watcher_quark (void);
+
+void nm_context_busy_watcher_integrate_source (GMainContext *outer_context,
+                                               GMainContext *inner_context,
+                                               GObject *context_busy_watcher);
+
+/*****************************************************************************/
+
+typedef struct {
+	GCancellable *cancellable;
+	GSource *cancel_on_idle_source;
+	gulong cancelled_id;
+	union {
+		struct {
+			GTask *task;
+		} async;
+		struct {
+			GMainLoop *main_loop;
+			GError **error_location;
+		} sync;
+	} data;
+	bool is_sync:1;
+} NMLInitData;
+
+NMLInitData *nml_init_data_new_sync (GCancellable *cancellable,
+                                     GMainLoop *main_loop,
+                                     GError **error_location);
+
+NMLInitData *nml_init_data_new_async (GCancellable *cancellable,
+                                      GTask *task_take);
+
+void nml_init_data_return (NMLInitData *init_data,
+                           GError *error_take);
+
+void nml_cleanup_context_busy_watcher_on_idle (GObject *context_busy_watcher_take,
+                                               GMainContext *context);
+
+/*****************************************************************************/
+
 typedef struct _NMLDBusObject     NMLDBusObject;
 typedef struct _NMLDBusObjWatcher NMLDBusObjWatcher;
 typedef struct _NMLDBusMetaIface  NMLDBusMetaIface;

libnm/nm-secret-agent-old.h

@@ -21,6 +21,7 @@ G_BEGIN_DECLS
 #define NM_SECRET_AGENT_OLD_AUTO_REGISTER       "auto-register"
 #define NM_SECRET_AGENT_OLD_REGISTERED          "registered"
 #define NM_SECRET_AGENT_OLD_CAPABILITIES        "capabilities"
+#define NM_SECRET_AGENT_OLD_DBUS_CONNECTION     "dbus-connection"
 
 /**
  * NMSecretAgentOld:
@@ -126,9 +127,12 @@ typedef struct {
 	/* Called when the subclass should retrieve and return secrets.  Subclass
 	 * must copy or reference any arguments it may require after returning from
 	 * this method, as the arguments will freed (except for 'self', 'callback',
-	 * and 'user_data' of course).  If the request is canceled, the callback
+	 * and 'user_data' of course).
-	 * should still be called, but with the
+	 *
-	 * NM_SECRET_AGENT_OLD_ERROR_AGENT_CANCELED error.
+	 * Before version 1.24, if the request is canceled, the callback
+	 * should still be called, but with the NM_SECRET_AGENT_ERROR_AGENT_CANCELED
+	 * error. Since 1.24, invoking the callback has no effect during cancellation
+	 * and may be omitted.
 	 */
 	void (*get_secrets) (NMSecretAgentOld *self,
 	                     NMConnection *connection,
@@ -140,10 +144,12 @@ typedef struct {
 	                     gpointer user_data);
 
 	/* Called when the subclass should cancel an outstanding request to
-	 * get secrets for a given connection.  Canceling the request MUST
+	 * get secrets for a given connection.
-	 * call the callback that was passed along with the initial get_secrets
+	 *
-	 * call, sending the NM_SECRET_AGENT_OLD_ERROR/
+	 * Before version 1.24, canceling the request MUST call the callback that was
-	 * NM_SECRET_AGENT_OLD_ERROR_AGENT_CANCELED error to that callback.
+	 * passed along with the initial get_secrets call, sending the NM_SECRET_AGENT_ERROR/
+	 * NM_SECRET_AGENT_ERROR_AGENT_CANCELED error to that callback. Since 1.24,
+	 * the get_secrets callback will be ignored during cancellation and may be omitted.
 	 */
 	void (*cancel_get_secrets) (NMSecretAgentOld *self,
 	                            const char *connection_path,
@@ -179,9 +185,26 @@ typedef struct {
 
 GType nm_secret_agent_old_get_type (void);
 
-gboolean nm_secret_agent_old_register        (NMSecretAgentOld *self,
+NM_AVAILABLE_IN_1_24
-                                              GCancellable *cancellable,
+GDBusConnection *nm_secret_agent_old_get_dbus_connection (NMSecretAgentOld *self);
-                                              GError **error);
+
+NM_AVAILABLE_IN_1_24
+GMainContext *nm_secret_agent_old_get_main_context (NMSecretAgentOld *self);
+
+NM_AVAILABLE_IN_1_24
+GObject *nm_secret_agent_old_get_context_busy_watcher (NMSecretAgentOld *self);
+
+NM_AVAILABLE_IN_1_24
+const char *nm_secret_agent_old_get_dbus_name_owner (NMSecretAgentOld *self);
+
+gboolean nm_secret_agent_old_get_registered (NMSecretAgentOld *self);
+
+/*****************************************************************************/
+
+NM_AVAILABLE_IN_1_24
+void nm_secret_agent_old_enable (NMSecretAgentOld *self,
+                                 gboolean enable);
+
 void     nm_secret_agent_old_register_async  (NMSecretAgentOld *self,
                                               GCancellable *cancellable,
                                               GAsyncReadyCallback callback,
@@ -190,18 +213,33 @@ gboolean nm_secret_agent_old_register_finish (NMSecretAgentOld *self,
                                               GAsyncResult *result,
                                               GError **error);
 
+NM_AVAILABLE_IN_1_24
+void nm_secret_agent_old_destroy (NMSecretAgentOld *self);
+
+/*****************************************************************************/
+
+NM_DEPRECATED_IN_1_24_FOR (nm_secret_agent_old_enable)
+gboolean nm_secret_agent_old_register        (NMSecretAgentOld *self,
+                                              GCancellable *cancellable,
+                                              GError **error);
+
+NM_DEPRECATED_IN_1_24_FOR (nm_secret_agent_old_enable)
 gboolean nm_secret_agent_old_unregister        (NMSecretAgentOld *self,
                                                 GCancellable *cancellable,
                                                 GError **error);
+
+NM_DEPRECATED_IN_1_24_FOR (nm_secret_agent_old_enable)
 void     nm_secret_agent_old_unregister_async  (NMSecretAgentOld *self,
                                                 GCancellable *cancellable,
                                                 GAsyncReadyCallback callback,
                                                 gpointer user_data);
+
+NM_DEPRECATED_IN_1_24_FOR (nm_secret_agent_old_enable)
 gboolean nm_secret_agent_old_unregister_finish (NMSecretAgentOld *self,
                                                 GAsyncResult *result,
                                                 GError **error);
 
-gboolean nm_secret_agent_old_get_registered (NMSecretAgentOld *self);
+/*****************************************************************************/
 
 void nm_secret_agent_old_get_secrets (NMSecretAgentOld *self,
                                       NMConnection *connection,

libnm/tests/test-remote-settings-client.c

@@ -62,7 +62,7 @@ test_add_connection (void)
 	                                add_cb,
 	                                &done);
 
-	nmtst_main_context_iterate_until (NULL, 5000, done);
+	nmtst_main_context_iterate_until_assert (NULL, 5000, done);
 
 	g_assert (gl.remote != NULL);
 
@@ -151,7 +151,7 @@ test_make_invisible (void)
 	                   set_visible_cb, NULL);
 
 	/* Wait for the connection to be removed */
-	nmtst_main_context_iterate_until (NULL, 5000, visible_changed && connection_removed);
+	nmtst_main_context_iterate_until_assert (NULL, 5000, visible_changed && connection_removed);
 
 	g_signal_handlers_disconnect_by_func (gl.remote, G_CALLBACK (visible_changed_cb), &visible_changed);
 	g_signal_handlers_disconnect_by_func (gl.client, G_CALLBACK (connection_removed_cb), &connection_removed);
@@ -225,7 +225,7 @@ test_make_visible (void)
 	                   set_visible_cb, NULL);
 
 	/* Wait for the settings service to announce the connection again */
-	nmtst_main_context_iterate_until (NULL, 5000, new);
+	nmtst_main_context_iterate_until_assert (NULL, 5000, new);
 
 	/* Ensure the new connection is the same as the one we made visible again */
 	g_assert (new == gl.remote);
@@ -313,7 +313,7 @@ test_remove_connection (void)
 	                   NULL,
 	                   deleted_cb, NULL);
 
-	nmtst_main_context_iterate_until (NULL, 5000, done && !gl.remote);
+	nmtst_main_context_iterate_until_assert (NULL, 5000, done && !gl.remote);
 
 	/* Ensure NMClient no longer has the connection */
 	conns = nm_client_get_connections (gl.client);
@@ -378,7 +378,7 @@ test_add_remove_connection (void)
 	                                add_remove_cb,
 	                                &done);
 
-	nmtst_main_context_iterate_until (NULL, 5000, done);
+	nmtst_main_context_iterate_until_assert (NULL, 5000, done);
 }
 
 /*****************************************************************************/
@@ -417,7 +417,7 @@ test_add_bad_connection (void)
 	                                &done);
 	g_clear_object (&connection);
 
-	nmtst_main_context_iterate_until (NULL, 5000, done);
+	nmtst_main_context_iterate_until_assert (NULL, 5000, done);
 	g_assert (gl.remote == NULL);
 }
 

libnm/tests/test-secret-agent.c

@@ -16,22 +16,17 @@
 
 enum {
 	SECRET_REQUESTED,
-
+	LAST_SIGNAL,
-	LAST_SIGNAL
 };
 
 static guint signals[LAST_SIGNAL] = { 0 };
 
-typedef NMSecretAgentOld TestSecretAgent;
+typedef NMSecretAgentOld      TestSecretAgent;
 typedef NMSecretAgentOldClass TestSecretAgentClass;
 
 GType test_secret_agent_get_type (void);
-G_DEFINE_TYPE (TestSecretAgent, test_secret_agent, NM_TYPE_SECRET_AGENT_OLD)
 
-static void
+G_DEFINE_TYPE (TestSecretAgent, test_secret_agent, NM_TYPE_SECRET_AGENT_OLD)
-test_secret_agent_init (TestSecretAgent *agent)
-{
-}
 
 static void
 test_secret_agent_get_secrets (NMSecretAgentOld                 *agent,
@@ -121,43 +116,42 @@ test_secret_agent_delete_secrets (NMSecretAgentOld                  *agent,
 	g_assert_not_reached ();
 }
 
+static void
+test_secret_agent_init (TestSecretAgent *agent)
+{
+}
+
+static NMSecretAgentOld *
+test_secret_agent_new (gboolean auto_register)
+{
+	return nmtstc_context_object_new (test_secret_agent_get_type (),
+	                                  TRUE,
+	                                  NM_SECRET_AGENT_OLD_IDENTIFIER, "test-secret-agent",
+	                                  NM_SECRET_AGENT_OLD_AUTO_REGISTER, auto_register,
+	                                  NULL);
+}
+
 static void
 test_secret_agent_class_init (TestSecretAgentClass *klass)
 {
 	GObjectClass *object_class = G_OBJECT_CLASS (klass);
 	NMSecretAgentOldClass *agent_class = NM_SECRET_AGENT_OLD_CLASS (klass);
 
-	agent_class->get_secrets = test_secret_agent_get_secrets;
+	agent_class->get_secrets =        test_secret_agent_get_secrets;
 	agent_class->cancel_get_secrets = test_secret_agent_cancel_get_secrets;
-	agent_class->save_secrets = test_secret_agent_save_secrets;
+	agent_class->save_secrets =       test_secret_agent_save_secrets;
-	agent_class->delete_secrets = test_secret_agent_delete_secrets;
+	agent_class->delete_secrets =     test_secret_agent_delete_secrets;
 
 	signals[SECRET_REQUESTED] =
-		g_signal_new ("secret-requested",
+	    g_signal_new ("secret-requested",
-		              G_OBJECT_CLASS_TYPE (object_class),
+	                  G_OBJECT_CLASS_TYPE (object_class),
-		              G_SIGNAL_RUN_LAST,
+	                  G_SIGNAL_RUN_LAST,
-		              0, NULL, NULL, NULL,
+	                  0, NULL, NULL, NULL,
-		              G_TYPE_STRING, 4,
+	                  G_TYPE_STRING, 4,
-		              NM_TYPE_CONNECTION,
+	                  NM_TYPE_CONNECTION,
-		              G_TYPE_STRING,
+	                  G_TYPE_STRING,
-		              G_TYPE_STRING,
+	                  G_TYPE_STRING,
-		              G_TYPE_STRING);
+	                  G_TYPE_STRING);
-
-}
-
-static NMSecretAgentOld *
-test_secret_agent_new (void)
-{
-	NMSecretAgentOld *agent;
-	GError *error = NULL;
-
-	agent = g_initable_new (test_secret_agent_get_type (), NULL, &error,
-	                        NM_SECRET_AGENT_OLD_IDENTIFIER, "test-secret-agent",
-	                        NM_SECRET_AGENT_OLD_AUTO_REGISTER, FALSE,
-	                        NULL);
-	g_assert_no_error (error);
-
-	return agent;
 }
 
 /*****************************************************************************/
@@ -171,7 +165,7 @@ typedef struct {
 	NMConnection *connection;
 
 	GMainLoop *loop;
-	guint timeout_id;
+	GSource *timeout_source;
 
 	char *ifname;
 	char *con_id;
@@ -179,12 +173,6 @@ typedef struct {
 	int secrets_requested;
 } TestSecretAgentData;
 
-static gboolean
-timeout_assert (gpointer user_data)
-{
-	g_assert_not_reached ();
-}
-
 static void
 connection_added_cb (GObject *s,
                      GAsyncResult *result,
@@ -221,33 +209,40 @@ register_cb (GObject *object, GAsyncResult *result, gpointer user_data)
 static void
 test_setup (TestSecretAgentData *sadata, gconstpointer test_data)
 {
-	static int counter = 0;
+	static int static_counter = 0;
-	const char *agent_notes = test_data;
+	const int counter = static_counter++;
+	const char *create_agent = test_data;
 	NMConnection *connection;
 	NMSettingConnection *s_con;
 	NMSettingWireless *s_wireless;
 	GBytes *ssid;
 	NMSetting *s_wsec;
-	GError *error = NULL;
+	gs_free_error GError *error = NULL;
 
 	sadata->sinfo = nmtstc_service_init ();
 	if (!sadata->sinfo)
 		return;
 
-	g_assert (g_main_context_get_thread_default () == NULL);
+	g_assert (nm_g_main_context_is_thread_default (NULL));
 
 	sadata->client = nmtstc_client_new (TRUE);
 
+	g_assert (nm_g_main_context_is_thread_default (NULL));
+	g_assert (nm_g_main_context_is_thread_default (nm_client_get_main_context (sadata->client)));
+
 	sadata->loop = g_main_loop_new (NULL, FALSE);
-	sadata->timeout_id = g_timeout_add_seconds (5, timeout_assert, NULL);
+
+	sadata->timeout_source = g_timeout_source_new_seconds (5);
+	g_source_set_callback (sadata->timeout_source, nmtst_g_source_assert_not_called, NULL, NULL);
+	g_source_attach (sadata->timeout_source, NULL);
 
 	sadata->ifname = g_strdup_printf ("wlan%d", counter);
 	sadata->con_id = g_strdup_printf ("%s-%d", TEST_CON_ID_PREFIX, counter);
-	counter++;
 
-	/* Create the device */
+	sadata->device = nmtstc_service_add_device (sadata->sinfo,
-	sadata->device = nmtstc_service_add_device (sadata->sinfo, sadata->client,
+	                                            sadata->client,
-	                                            "AddWifiDevice", sadata->ifname);
+	                                            "AddWifiDevice",
+	                                            sadata->ifname);
 
 	/* Create the connection */
 	connection = nmtst_create_minimal_connection (sadata->con_id, NULL, NM_SETTING_WIRELESS_SETTING_NAME, &s_con);
@@ -278,18 +273,25 @@ test_setup (TestSecretAgentData *sadata, gconstpointer test_data)
 	g_main_loop_run (sadata->loop);
 	g_assert (sadata->connection);
 
-	if (agent_notes) {
+	if (nm_streq (create_agent, "1")) {
-		sadata->agent = test_secret_agent_new ();
+		gboolean auto_register = nmtst_get_rand_bool ();
 
-		if (!strcmp (agent_notes, "sync")) {
+		sadata->agent = test_secret_agent_new (auto_register);
+
+		if (auto_register) {
+			g_assert (nm_secret_agent_old_get_registered (sadata->agent));
 			nm_secret_agent_old_register (sadata->agent, NULL, &error);
 			g_assert_no_error (error);
-			g_assert (nm_secret_agent_old_get_registered (sadata->agent));
 		} else {
-			nm_secret_agent_old_register_async (sadata->agent, NULL,
+			g_assert (!nm_secret_agent_old_get_registered (sadata->agent));
-			                                register_cb, sadata);
+			nm_secret_agent_old_register_async (sadata->agent,
+			                                    NULL,
+			                                    register_cb,
+			                                    sadata);
 			g_main_loop_run (sadata->loop);
 		}
+
+		g_assert (nm_secret_agent_old_get_registered (sadata->agent));
 	}
 }
 
@@ -298,11 +300,22 @@ test_cleanup (TestSecretAgentData *sadata, gconstpointer test_data)
 {
 	GVariant *ret;
 	GError *error = NULL;
+	NMTstContextBusyWatcherData watcher_data = { };
+
+	g_assert (nm_g_main_context_is_thread_default (NULL));
 
 	if (!sadata->sinfo)
 		return;
 
+	g_assert (nm_g_main_context_is_thread_default (nm_client_get_main_context (sadata->client)));
+
+	nmtst_context_busy_watcher_add (&watcher_data,
+	                                nm_client_get_context_busy_watcher (sadata->client));
+
 	if (sadata->agent) {
+		nmtst_context_busy_watcher_add (&watcher_data,
+		                                nm_secret_agent_old_get_context_busy_watcher (sadata->agent));
+
 		if (nm_secret_agent_old_get_registered (sadata->agent)) {
 			nm_secret_agent_old_unregister (sadata->agent, NULL, &error);
 			g_assert_no_error (error);
@@ -325,11 +338,21 @@ test_cleanup (TestSecretAgentData *sadata, gconstpointer test_data)
 
 	nmtstc_service_cleanup (sadata->sinfo);
 
-	g_source_remove (sadata->timeout_id);
+	nm_clear_g_source_inst (&sadata->timeout_source);
+
 	g_main_loop_unref (sadata->loop);
 
 	g_free (sadata->ifname);
 	g_free (sadata->con_id);
+
+	*sadata = (TestSecretAgentData) { };
+
+	nmtst_context_busy_watcher_wait (&watcher_data);
+
+	while (g_main_context_iteration (NULL, FALSE)) {
+	}
+
+	nmtst_main_context_assert_no_dispatch (NULL, nmtst_get_rand_uint32 () % 500);
 }
 
 /*****************************************************************************/
@@ -529,17 +552,18 @@ test_secret_agent_good (TestSecretAgentData *sadata, gconstpointer test_data)
 	g_assert_cmpint (sadata->secrets_requested, ==, 1);
 }
 
+/*****************************************************************************/
+
 static void
 async_init_cb (GObject *object, GAsyncResult *result, gpointer user_data)
 {
 	GMainLoop *loop = user_data;
-	GError *error = NULL;
+	gs_free_error GError *error = NULL;
-	GObject *agent;
+	gs_unref_object GObject *agent = NULL;
 
 	agent = g_async_initable_new_finish (G_ASYNC_INITABLE (object), result, &error);
-	g_assert_error (error, NM_SECRET_AGENT_ERROR, NM_SECRET_AGENT_ERROR_FAILED);
+	nmtst_assert_success (NM_IS_SECRET_AGENT_OLD (agent), error);
-	g_assert (agent == NULL);
+	g_assert (!nm_secret_agent_old_get_registered (NM_SECRET_AGENT_OLD (agent)));
-	g_clear_error (&error);
 
 	g_main_loop_quit (loop);
 }
@@ -547,89 +571,151 @@ async_init_cb (GObject *object, GAsyncResult *result, gpointer user_data)
 static void
 test_secret_agent_nm_not_running (void)
 {
-	NMSecretAgentOld *agent;
+	gs_unref_object NMSecretAgentOld *agent = NULL;
-	GMainLoop *loop;
+	nm_auto_unref_gmainloop GMainLoop *loop = NULL;
 	GError *error = NULL;
 
-	agent = g_initable_new (test_secret_agent_get_type (), NULL, &error,
+	agent = g_initable_new (test_secret_agent_get_type (),
+	                        NULL,
+	                        &error,
 	                        NM_SECRET_AGENT_OLD_IDENTIFIER, "test-secret-agent",
 	                        NULL);
-	g_assert_error (error, NM_SECRET_AGENT_ERROR, NM_SECRET_AGENT_ERROR_FAILED);
+	nmtst_assert_success (NM_IS_SECRET_AGENT_OLD (agent), error);
-	g_assert (agent == NULL);
+	g_assert (!nm_secret_agent_old_get_registered (agent));
-	g_clear_error (&error);
+	g_clear_object (&agent);
 
 	loop = g_main_loop_new (NULL, FALSE);
 	g_async_initable_new_async (test_secret_agent_get_type (),
 	                            G_PRIORITY_DEFAULT,
-	                            NULL, async_init_cb, loop,
+	                            NULL,
+	                            async_init_cb,
+	                            loop,
 	                            NM_SECRET_AGENT_OLD_IDENTIFIER, "test-secret-agent",
 	                            NULL);
 	g_main_loop_run (loop);
-	g_main_loop_unref (loop);
 }
 
+/*****************************************************************************/
+
+typedef struct {
+	int step;
+	int invoke_count;
+} AutoRegisterData;
+
 static void
 registered_changed (GObject *object, GParamSpec *pspec, gpointer user_data)
 {
-	GMainLoop *loop = user_data;
+	NMSecretAgentOld *agent = NM_SECRET_AGENT_OLD (object);
+	AutoRegisterData *data = user_data;
 
-	g_main_loop_quit (loop);
+	g_assert (data);
+	g_assert (NM_IS_SECRET_AGENT_OLD (agent));
+
+	data->invoke_count++;
+	g_assert_cmpint (data->invoke_count, ==, data->step);
+
+	switch (data->step) {
+	case 1:
+	case 3:
+		g_assert (nm_secret_agent_old_get_registered (agent));
+		break;
+	case 2:
+	case 4:
+		g_assert (!nm_secret_agent_old_get_registered (agent));
+		break;
+	default:
+		g_assert_not_reached ();
+	}
 }
 
 static void
 test_secret_agent_auto_register (void)
 {
 	NMTstcServiceInfo *sinfo;
-	NMSecretAgentOld *agent;
+	gs_unref_object NMSecretAgentOld *agent = NULL;
-	GMainLoop *loop;
 	GError *error = NULL;
+	AutoRegisterData auto_register_data = {
+		.step         = 0,
+		.invoke_count = 0,
+	};
+	gulong signal_id;
+	NMTstContextBusyWatcherData watcher_data = { };
 
 	sinfo = nmtstc_service_init ();
 	if (!nmtstc_service_available (sinfo))
 		return;
 
-	loop = g_main_loop_new (NULL, FALSE);
+	agent = test_secret_agent_new (FALSE);
-
-	agent = test_secret_agent_new ();
-	g_object_set (agent,
-	              NM_SECRET_AGENT_OLD_AUTO_REGISTER, TRUE,
-	              NULL);
-	g_signal_connect (agent, "notify::" NM_SECRET_AGENT_OLD_REGISTERED,
-	                  G_CALLBACK (registered_changed), loop);
-
 	g_assert (!nm_secret_agent_old_get_registered (agent));
+
+	signal_id = g_signal_connect (agent, "notify::" NM_SECRET_AGENT_OLD_REGISTERED,
+	                              G_CALLBACK (registered_changed), &auto_register_data);
+
+	if (nmtst_get_rand_bool ()) {
+		g_object_set (agent,
+		              NM_SECRET_AGENT_OLD_AUTO_REGISTER, TRUE,
+		              NULL);
+	} else
+		nm_secret_agent_old_enable (agent, TRUE);
+	g_assert (!nm_secret_agent_old_get_registered (agent));
+
 	nm_secret_agent_old_register (agent, NULL, &error);
 	g_assert_no_error (error);
-	g_assert (nm_secret_agent_old_get_registered (agent));
-
-	/* The GLib ObjectManager doesn't like when we drop the service
-	 * in between it sees the service disappear and the call to
-	 * GetManagedObjects. Give it a chance to do its business.
-	 * Arguably a bug. */
-	g_main_context_iteration (NULL, FALSE);
-
-	/* Shut down test service */
-	nmtstc_service_cleanup (sinfo);
-	g_main_loop_run (loop);
 	g_assert (!nm_secret_agent_old_get_registered (agent));
 
-	/* Restart test service */
+	auto_register_data.step = 1;
+	nmtst_main_context_iterate_until_assert (NULL,
+	                                         1000,
+	                                         nm_secret_agent_old_get_registered (agent));
+
+	auto_register_data.step = 2;
+	nm_secret_agent_old_enable (agent, FALSE);
+	g_assert (!nm_secret_agent_old_get_registered (agent));
+
+	nmtst_main_context_iterate_until (NULL,
+	                                  nmtst_get_rand_uint32 () % 200,
+	                                  FALSE);
+
+	g_assert (!nm_secret_agent_old_get_registered (agent));
+
+	nmtstc_service_cleanup (sinfo);
+
+	g_assert (!nm_secret_agent_old_get_registered (agent));
+
+	nm_secret_agent_old_enable (agent, TRUE);
+
+	g_assert (!nm_secret_agent_old_get_registered (agent));
+
+	nmtst_main_context_iterate_until (NULL,
+	                                  nmtst_get_rand_uint32 () % 200,
+	                                  FALSE);
+
+	g_assert (!nm_secret_agent_old_get_registered (agent));
+
 	sinfo = nmtstc_service_init ();
 	g_assert (nmtstc_service_available (sinfo));
 
-	g_main_loop_run (loop);
+	auto_register_data.step = 3;
-	g_assert (nm_secret_agent_old_get_registered (agent));
+	nmtst_main_context_iterate_until_assert (NULL,
+	                                         1000,
+	                                         nm_secret_agent_old_get_registered (agent));
 
-	/* Let ObjectManager initialize (see above). */
-	g_main_context_iteration (NULL, FALSE);
-
-	/* Shut down test service again */
 	nmtstc_service_cleanup (sinfo);
-	g_main_loop_run (loop);
-	g_assert (!nm_secret_agent_old_get_registered (agent));
 
-	g_object_unref (agent);
+	auto_register_data.step = 4;
-	g_main_loop_unref (loop);
+	nmtst_main_context_iterate_until_assert (NULL,
+	                                         1000,
+	                                         !nm_secret_agent_old_get_registered (agent));
+
+	nm_clear_g_signal_handler (agent, &signal_id);
+
+	nmtst_context_busy_watcher_add (&watcher_data, nm_secret_agent_old_get_context_busy_watcher (agent));
+
+	g_clear_object (&agent);
+
+	nmtst_context_busy_watcher_wait (&watcher_data);
+
+	nmtst_main_context_assert_no_dispatch (NULL, nmtst_get_rand_uint32 () % 500);
 }
 
 /*****************************************************************************/
@@ -639,25 +725,16 @@ NMTST_DEFINE ();
 int
 main (int argc, char **argv)
 {
-	int ret;
-
 	g_setenv ("LIBNM_USE_SESSION_BUS", "1", TRUE);
 
 	nmtst_init (&argc, &argv, TRUE);
 
-	g_test_add ("/libnm/secret-agent/none", TestSecretAgentData, NULL,
+	g_test_add ("/libnm/secret-agent/none",       TestSecretAgentData, "0", test_setup, test_secret_agent_none,       test_cleanup);
-	            test_setup, test_secret_agent_none, test_cleanup);
+	g_test_add ("/libnm/secret-agent/no-secrets", TestSecretAgentData, "1", test_setup, test_secret_agent_no_secrets, test_cleanup);
-	g_test_add ("/libnm/secret-agent/no-secrets", TestSecretAgentData, "sync",
+	g_test_add ("/libnm/secret-agent/cancel",     TestSecretAgentData, "1", test_setup, test_secret_agent_cancel,     test_cleanup);
-	            test_setup, test_secret_agent_no_secrets, test_cleanup);
+	g_test_add ("/libnm/secret-agent/good",       TestSecretAgentData, "1", test_setup, test_secret_agent_good,       test_cleanup);
-	g_test_add ("/libnm/secret-agent/cancel", TestSecretAgentData, "async",
-	            test_setup, test_secret_agent_cancel, test_cleanup);
-	g_test_add ("/libnm/secret-agent/good", TestSecretAgentData, "async",
-	            test_setup, test_secret_agent_good, test_cleanup);
 	g_test_add_func ("/libnm/secret-agent/nm-not-running", test_secret_agent_nm_not_running);
 	g_test_add_func ("/libnm/secret-agent/auto-register", test_secret_agent_auto_register);
 
-	ret = g_test_run ();
+	return g_test_run ();
-
-	return ret;
 }
-

po/POTFILES.in

@@ -134,6 +134,7 @@ libnm/nm-device-wpan.c
 libnm/nm-device.c
 libnm/nm-object.c
 libnm/nm-remote-connection.c
+libnm/nm-secret-agent-old.c
 libnm/nm-vpn-plugin-old.c
 libnm/nm-vpn-service-plugin.c
 data/org.freedesktop.NetworkManager.policy.in.in

shared/nm-glib-aux/nm-shared-utils.h

@@ -1026,6 +1026,25 @@ nm_g_main_context_push_thread_default (GMainContext *context)
 	return context;
 }
 
+static inline gboolean
+nm_g_main_context_is_thread_default (GMainContext *context)
+{
+	GMainContext *cur_context;
+
+	cur_context = g_main_context_get_thread_default ();
+	if (cur_context == context)
+		return TRUE;
+
+	if (G_UNLIKELY (!cur_context))
+		cur_context = g_main_context_default ();
+	else if (G_UNLIKELY (!context))
+		context = g_main_context_default ();
+	else
+		return FALSE;
+
+	return (cur_context == context);
+}
+
 static inline GMainContext *
 nm_g_main_context_push_thread_default_if_necessary (GMainContext *context)
 {
@@ -1198,8 +1217,9 @@ _nm_utils_strv_equal (char **strv1, char **strv2)
 /*****************************************************************************/
 
 #define NM_UTILS_NSEC_PER_SEC  ((gint64) 1000000000)
-#define NM_UTILS_NSEC_PER_MSEC ((gint64) 1000000)
+#define NM_UTILS_USEC_PER_SEC  ((gint64) 1000000)
 #define NM_UTILS_MSEC_PER_SEC  ((gint64) 1000)
+#define NM_UTILS_NSEC_PER_MSEC ((gint64) 1000000)
 
 static inline gint64
 NM_UTILS_NSEC_TO_MSEC_CEIL (gint64 nsec)

shared/nm-test-libnm-utils.h

@@ -67,4 +67,18 @@ void nmtstc_service_update_connection_variant (NMTstcServiceInfo *sinfo,
                                                GVariant *connection,
                                                gboolean verify_connection);
 
-NMClient *nmtstc_client_new (gboolean allow_iterate_main_context);
+gpointer nmtstc_context_object_new_valist (GType gtype,
+                                           gboolean allow_iterate_main_context,
+                                           const char *first_property_name,
+                                           va_list var_args);
+
+gpointer nmtstc_context_object_new (GType gtype,
+                                    gboolean allow_iterate_main_context,
+                                    const char *first_property_name,
+                                    ...);
+
+static inline NMClient *
+nmtstc_client_new (gboolean allow_iterate_main_context)
+{
+	return nmtstc_context_object_new (NM_TYPE_CLIENT, allow_iterate_main_context, NULL);
+}

shared/nm-test-utils-impl.c

@@ -216,7 +216,7 @@ again_wait:
 typedef struct {
 	GMainLoop *loop;
 	const char *ifname;
-	char *path;
+	const char *path;
 	NMDevice *device;
 } AddDeviceInfo;
 
@@ -227,21 +227,17 @@ device_added_cb (NMClient *client,
 {
 	AddDeviceInfo *info = user_data;
 
-	g_assert (device);
+	g_assert (info);
+	g_assert (!info->device);
+
+	g_assert (NM_IS_DEVICE (device));
 	g_assert_cmpstr (nm_object_get_path (NM_OBJECT (device)), ==, info->path);
 	g_assert_cmpstr (nm_device_get_iface (device), ==, info->ifname);
 
-	info->device = device;
+	info->device = g_object_ref (device);
 	g_main_loop_quit (info->loop);
 }
 
-static gboolean
-timeout (gpointer user_data)
-{
-	g_assert_not_reached ();
-	return G_SOURCE_REMOVE;
-}
-
 static GVariant *
 call_add_wired_device (GDBusProxy *proxy, const char *ifname, const char *hwaddr,
                        const char **subchannels, GError **error)
@@ -275,39 +271,53 @@ call_add_device (GDBusProxy *proxy, const char *method, const char *ifname, GErr
 }
 
 static NMDevice *
-add_device_common (NMTstcServiceInfo *sinfo, NMClient *client,
+add_device_common (NMTstcServiceInfo *sinfo,
-                   const char *method, const char *ifname,
+                   NMClient *client,
-                   const char *hwaddr, const char **subchannels)
+                   const char *method,
+                   const char *ifname,
+                   const char *hwaddr,
+                   const char **subchannels)
 {
+	nm_auto_unref_gmainloop GMainLoop *loop = NULL;
+	gs_unref_variant GVariant *ret = NULL;
+	gs_free_error GError *error = NULL;
 	AddDeviceInfo info;
-	GError *error = NULL;
-	GVariant *ret;
-	guint timeout_id;
 
-	if (g_strcmp0 (method, "AddWiredDevice") == 0)
+	g_assert (sinfo);
+	g_assert (NM_IS_CLIENT (client));
+
+	if (nm_streq0 (method, "AddWiredDevice"))
 		ret = call_add_wired_device (sinfo->proxy, ifname, hwaddr, subchannels, &error);
 	else
 		ret = call_add_device (sinfo->proxy, method, ifname, &error);
 
-	g_assert_no_error (error);
+	nmtst_assert_success (ret, error);
-	g_assert (ret);
 	g_assert_cmpstr (g_variant_get_type_string (ret), ==, "(o)");
-	g_variant_get (ret, "(o)", &info.path);
-	g_variant_unref (ret);
 
-	/* Wait for libnm to find the device */
+	/* Wait for NMClient to find the device */
-	info.ifname = ifname;
+
-	info.loop = g_main_loop_new (NULL, FALSE);
+	loop = g_main_loop_new (nm_client_get_main_context (client), FALSE);
-	g_signal_connect (client, "device-added",
+
-	                  G_CALLBACK (device_added_cb), &info);
+	info = (AddDeviceInfo) {
-	timeout_id = g_timeout_add_seconds (5, timeout, NULL);
+		.ifname = ifname,
-	g_main_loop_run (info.loop);
+		.loop   = loop,
+	};
+	g_variant_get (ret, "(&o)", &info.path);
+
+	g_signal_connect (client,
+	                  NM_CLIENT_DEVICE_ADDED,
+	                  G_CALLBACK (device_added_cb),
+	                  &info);
+
+	if (!nmtst_main_loop_run (loop, 5000))
+		g_assert_not_reached ();
 
-	g_source_remove (timeout_id);
 	g_signal_handlers_disconnect_by_func (client, device_added_cb, &info);
-	g_free (info.path);
-	g_main_loop_unref (info.loop);
 
+	g_assert (NM_IS_DEVICE (info.device));
+
+	g_assert (info.device == nm_client_get_device_by_path (client, nm_object_get_path (NM_OBJECT (info.device))));
+	g_object_unref (info.device);
 	return info.device;
 }
 
@@ -410,35 +420,47 @@ nmtstc_service_update_connection_variant (NMTstcServiceInfo *sinfo,
 /*****************************************************************************/
 
 typedef struct {
+	GType gtype;
 	GMainLoop *loop;
-	NMClient *client;
+	GObject *obj;
-} NMTstcClientNewData;
+	bool call_nm_client_new_async:1;
+} NMTstcObjNewData;
 
 static void
-_nmtstc_client_new_cb (GObject *source_object,
+_context_object_new_do_cb (GObject *source_object,
-                       GAsyncResult *res,
+                           GAsyncResult *res,
-                       gpointer user_data)
+                           gpointer user_data)
 {
-	NMTstcClientNewData *d = user_data;
+	NMTstcObjNewData *d = user_data;
 	gs_free_error GError *error = NULL;
 
-	g_assert (!d->client);
+	g_assert (!d->obj);
 
-	d->client = nm_client_new_finish (res,
+	if (d->call_nm_client_new_async) {
-	                                  nmtst_get_rand_bool () ? &error : NULL);
+		d->obj = G_OBJECT (nm_client_new_finish (res,
+		                                         nmtst_get_rand_bool () ? &error : NULL));
+	} else {
+		d->obj = g_async_initable_new_finish (G_ASYNC_INITABLE (source_object),
+		                                      res,
+		                                      nmtst_get_rand_bool () ? &error : NULL);
+	}
 
-	nmtst_assert_success (NM_IS_CLIENT (d->client), error);
+	nmtst_assert_success (G_IS_OBJECT (d->obj), error);
+	g_assert (G_OBJECT_TYPE (d->obj) == d->gtype);
 
 	g_main_loop_quit (d->loop);
 }
 
-static NMClient *
+static GObject *
-_nmtstc_client_new (gboolean sync)
+_context_object_new_do (GType gtype,
+                        gboolean sync,
+                        const gchar *first_property_name,
+                        va_list var_args)
 {
 	gs_free_error GError *error = NULL;
-	NMClient *client;
+	GObject *obj;
 
-	/* Create a NMClient instance synchronously, and arbitrarily use either
+	/* Create a GObject instance synchronously, and arbitrarily use either
 	 * the sync or async constructor.
 	 *
 	 * Note that the sync and async construct differ in one important aspect:
@@ -456,125 +478,128 @@ _nmtstc_client_new (gboolean sync)
 			g_source_attach (source, g_main_context_get_thread_default ());
 		}
 
-		if (nmtst_get_rand_bool ()) {
+		if (   gtype != NM_TYPE_CLIENT
+		    || first_property_name
+		    || nmtst_get_rand_bool ()) {
 			gboolean success;
 
-			client = g_object_new (NM_TYPE_CLIENT, NULL);
+			if (   first_property_name
-			g_assert (NM_IS_CLIENT (client));
+			    || nmtst_get_rand_bool ())
+				obj = g_object_new_valist (gtype, first_property_name, var_args);
+			else
+				obj = g_object_new (gtype, NULL);
 
-			success = g_initable_init (G_INITABLE (client),
+			success = g_initable_init (G_INITABLE (obj),
 			                           NULL,
 			                           nmtst_get_rand_bool () ? &error : NULL);
 			nmtst_assert_success (success, error);
 		} else {
-			client = nm_client_new (NULL,
+			obj = G_OBJECT (nm_client_new (NULL,
-			                        nmtst_get_rand_bool () ? &error : NULL);
+			                               nmtst_get_rand_bool () ? &error : NULL));
 		}
 	} else {
 		nm_auto_unref_gmainloop GMainLoop *loop = NULL;
-		NMTstcClientNewData d = { .loop = NULL, };
+		NMTstcObjNewData d = {
+			.gtype                    = gtype,
+			.loop                     = NULL,
+		};
+		gs_unref_object GObject *obj2 = NULL;
 
 		loop = g_main_loop_new (g_main_context_get_thread_default (), FALSE);
-
 		d.loop = loop;
-		nm_client_new_async (NULL,
+
-		                     _nmtstc_client_new_cb,
+		if (   gtype != NM_TYPE_CLIENT
-		                     &d);
+		    || first_property_name
+		    || nmtst_get_rand_bool ()) {
+			if (   first_property_name
+			    || nmtst_get_rand_bool ())
+				obj2 = g_object_new_valist (gtype, first_property_name, var_args);
+			else
+				obj2 = g_object_new (gtype, NULL);
+
+			g_async_initable_init_async (G_ASYNC_INITABLE (obj2),
+			                             G_PRIORITY_DEFAULT,
+			                             NULL,
+			                             _context_object_new_do_cb,
+			                             &d);
+		} else {
+			d.call_nm_client_new_async = TRUE;
+			nm_client_new_async (NULL,
+			                     _context_object_new_do_cb,
+			                     &d);
+		}
 		g_main_loop_run (loop);
-		g_assert (NM_IS_CLIENT (d.client));
+		obj = d.obj;
-		client = d.client;
+		g_assert (!obj2 || obj == obj2);
 	}
 
-	nmtst_assert_success (NM_IS_CLIENT (client), error);
+	nmtst_assert_success (G_IS_OBJECT (obj), error);
-	return client;
+	g_assert (G_OBJECT_TYPE (obj) == gtype);
+	return obj;
 }
 
 typedef struct {
+	GType gtype;
+	const char *first_property_name;
+	va_list var_args;
 	GMainLoop *loop;
-	NMClient *client;
+	GObject *obj;
 	bool sync;
 } NewSyncInsideDispatchedData;
 
 static gboolean
-_nmtstc_client_new_inside_loop_do (gpointer user_data)
+_context_object_new_inside_loop_do (gpointer user_data)
 {
 	NewSyncInsideDispatchedData *d = user_data;
 
 	g_assert (d->loop);
-	g_assert (!d->client);
+	g_assert (!d->obj);
 
-	d->client = nmtstc_client_new (d->sync);
+	d->obj = nmtstc_context_object_new_valist (d->gtype, d->sync, d->first_property_name, d->var_args);
 	g_main_loop_quit (d->loop);
 	return G_SOURCE_CONTINUE;
 }
 
-static NMClient *
+static GObject *
-_nmtstc_client_new_inside_loop (gboolean sync)
+_context_object_new_inside_loop (GType gtype,
+                                 gboolean sync,
+                                 const char *first_property_name,
+                                 va_list var_args)
 {
 	GMainContext *context = g_main_context_get_thread_default ();
 	nm_auto_unref_gmainloop GMainLoop *loop = g_main_loop_new (context, FALSE);
 	NewSyncInsideDispatchedData d = {
-		.sync = sync,
+		.gtype               = gtype,
-		.loop = loop,
+		.first_property_name = first_property_name,
+		.sync                = sync,
+		.loop                = loop,
 	};
 	nm_auto_destroy_and_unref_gsource GSource *source = NULL;
 
+	va_copy (d.var_args, var_args);
+
 	source = g_idle_source_new ();
-	g_source_set_callback (source, _nmtstc_client_new_inside_loop_do, &d, NULL);
+	g_source_set_callback (source, _context_object_new_inside_loop_do, &d, NULL);
 	g_source_attach (source, context);
 
 	g_main_loop_run (loop);
-	g_assert (NM_IS_CLIENT (d.client));
+
-	return d.client;
+	va_end (d.var_args);
+
+	g_assert (G_IS_OBJECT (d.obj));
+	g_assert (G_OBJECT_TYPE (d.obj) == gtype);
+	return d.obj;
 }
 
-static NMClient *
+gpointer
-_nmtstc_client_new_extra_context (void)
+nmtstc_context_object_new_valist (GType gtype,
-{
+                                  gboolean allow_iterate_main_context,
-	GMainContext *inner_context;
+                                  const char *first_property_name,
-	NMClient *client;
+                                  va_list var_args)
-	GSource *source;
-	guint key_idx;
-
-	inner_context = g_main_context_new ();
-	g_main_context_push_thread_default (inner_context);
-
-	client = nmtstc_client_new (TRUE);
-
-	source = nm_utils_g_main_context_create_integrate_source (inner_context);
-
-	g_main_context_pop_thread_default (inner_context);
-	g_main_context_unref (inner_context);
-
-	g_source_attach (source, g_main_context_get_thread_default ());
-
-	for (key_idx = 0; TRUE; key_idx++) {
-		char s[100];
-
-		/* nmtstc_client_new() may call _nmtstc_client_new_extra_context() repeatedly. We
-		 * need to attach the source to a previously unused key. */
-		nm_sprintf_buf (s, "nm-test-extra-context-%u", key_idx);
-		if (!g_object_get_data (G_OBJECT (client), s)) {
-			g_object_set_data_full (G_OBJECT (client),
-			                        s,
-			                        source,
-			                        (GDestroyNotify) nm_g_source_destroy_and_unref);
-			break;
-		}
-	}
-
-	return client;
-}
-
-NMClient *
-nmtstc_client_new (gboolean allow_iterate_main_context)
 {
 	gboolean inside_loop;
 	gboolean sync;
 
-	if (nmtst_get_rand_uint32 () % 5 == 0)
-		return _nmtstc_client_new_extra_context ();
-
 	if (!allow_iterate_main_context) {
 		sync = TRUE;
 		inside_loop = FALSE;
@@ -587,11 +612,26 @@ nmtstc_client_new (gboolean allow_iterate_main_context)
 	}
 
 	if (inside_loop) {
-		/* Create the client on an idle handler of the current context.
+		/* Create the obj on an idle handler of the current context.
 		 * In practice, it should make no difference, which this check
 		 * tries to prove. */
-		return _nmtstc_client_new_inside_loop (sync);
+		return _context_object_new_inside_loop (gtype, sync, first_property_name, var_args);
 	}
 
-	return _nmtstc_client_new (sync);
+	return _context_object_new_do (gtype, sync, first_property_name, var_args);
+}
+
+gpointer
+nmtstc_context_object_new (GType gtype,
+                           gboolean allow_iterate_main_context,
+                           const char *first_property_name,
+                           ...)
+{
+	GObject *obj;
+	va_list var_args;
+
+	va_start (var_args, first_property_name);
+	obj = nmtstc_context_object_new_valist (gtype, allow_iterate_main_context, first_property_name, var_args);
+	va_end (var_args);
+	return obj;
 }

shared/nm-utils/nm-test-utils.h

@@ -967,6 +967,26 @@ nmtst_rand_perm_gslist (GRand *rand, GSList *list)
 
 /*****************************************************************************/
 
+static inline gboolean
+nmtst_g_source_assert_not_called (gpointer user_data)
+{
+	g_assert_not_reached ();
+	return G_SOURCE_CONTINUE;
+}
+
+static inline gboolean
+nmtst_g_source_set_boolean_true (gpointer user_data)
+{
+	gboolean *ptr = user_data;
+
+	g_assert (ptr);
+	g_assert (!*ptr);
+	*ptr = TRUE;
+	return G_SOURCE_CONTINUE;
+}
+
+/*****************************************************************************/
+
 static inline gboolean
 _nmtst_main_loop_run_timeout (gpointer user_data)
 {
@@ -1010,36 +1030,123 @@ _nmtst_main_loop_quit_on_notify (GObject *object, GParamSpec *pspec, gpointer us
 }
 #define nmtst_main_loop_quit_on_notify ((GCallback) _nmtst_main_loop_quit_on_notify)
 
-static inline gboolean
-_nmtst_main_context_iterate_until_timeout (gpointer user_data)
-{
-	gboolean *p_had_pointer = user_data;
-
-	g_assert (!*p_had_pointer);
-	*p_had_pointer = TRUE;
-	return G_SOURCE_CONTINUE;
-}
-
 #define nmtst_main_context_iterate_until(context, timeout_msec, condition) \
-	G_STMT_START { \
+	({ \
 		nm_auto_destroy_and_unref_gsource GSource *_source = NULL; \
 		GMainContext *_context = (context); \
 		gboolean _had_timeout = FALSE; \
 		\
 		_source = g_timeout_source_new (timeout_msec); \
-		g_source_set_callback (_source, _nmtst_main_context_iterate_until_timeout, &_had_timeout, NULL); \
+		g_source_set_callback (_source, nmtst_g_source_set_boolean_true, &_had_timeout, NULL); \
 		g_source_attach (_source, _context); \
 		\
 		while (TRUE) { \
 			if (condition) \
 				break; \
 			g_main_context_iteration (_context, TRUE); \
-			g_assert (!_had_timeout && #condition); \
+			if (_had_timeout) \
+				break; \
 		} \
+		\
+		!_had_timeout; \
+	})
+
+#define nmtst_main_context_iterate_until_assert(context, timeout_msec, condition) \
+	G_STMT_START { \
+		if (!nmtst_main_context_iterate_until (context, timeout_msec, condition)) \
+			g_assert (FALSE && #condition); \
 	} G_STMT_END
 
 /*****************************************************************************/
 
+static inline void
+nmtst_main_context_assert_no_dispatch (GMainContext *context,
+                                       guint timeout_msec)
+{
+	nm_auto_destroy_and_unref_gsource GSource *source = NULL;
+	gboolean timeout_hit = FALSE;
+
+	source = g_timeout_source_new (timeout_msec);
+	g_source_set_callback (source, nmtst_g_source_set_boolean_true, &timeout_hit, NULL);
+	g_source_attach (source, context);
+
+	while (g_main_context_iteration (context, TRUE)) {
+		if (timeout_hit)
+			return;
+		g_assert_not_reached ();
+	}
+}
+
+/*****************************************************************************/
+
+typedef struct {
+	GMainLoop *_main_loop;
+	union {
+		GSList *_list;
+		const void *const is_waiting;
+	};
+} NMTstContextBusyWatcherData;
+
+static inline void
+_nmtst_context_busy_watcher_add_cb (gpointer data,
+                                    GObject *where_the_object_was)
+{
+	NMTstContextBusyWatcherData *watcher_data = data;
+	GSList *l;
+
+	g_assert (watcher_data);
+
+	l = g_slist_find (watcher_data->_list, where_the_object_was);
+	g_assert (l);
+
+	watcher_data->_list = g_slist_delete_link (watcher_data->_list, l);
+	if (!watcher_data->_list)
+		g_main_loop_quit (watcher_data->_main_loop);
+}
+
+static inline void
+nmtst_context_busy_watcher_add (NMTstContextBusyWatcherData *watcher_data,
+                                GObject *object)
+{
+	g_assert (watcher_data);
+	g_assert (G_IS_OBJECT (object));
+
+	if (!watcher_data->_main_loop) {
+		watcher_data->_main_loop = g_main_loop_new (g_main_context_get_thread_default (),
+		                                            FALSE);
+		g_assert (!watcher_data->_list);
+	} else {
+		g_assert (   g_main_loop_get_context (watcher_data->_main_loop)
+		          == (g_main_context_get_thread_default () ?: g_main_context_default ()));
+	}
+
+	g_object_weak_ref (object,
+	                   _nmtst_context_busy_watcher_add_cb,
+	                   watcher_data);
+	watcher_data->_list = g_slist_prepend (watcher_data->_list, object);
+}
+
+static inline void
+nmtst_context_busy_watcher_wait (NMTstContextBusyWatcherData *watcher_data)
+{
+	g_assert (watcher_data);
+
+	if (!watcher_data->_main_loop) {
+		g_assert (!watcher_data->_list);
+		return;
+	}
+
+	if (watcher_data->_list) {
+		if (!nmtst_main_loop_run (watcher_data->_main_loop, 5000))
+			g_error ("timeout running mainloop waiting for GObject to destruct");
+	}
+
+	g_assert (!watcher_data->_list);
+	nm_clear_pointer (&watcher_data->_main_loop, g_main_loop_unref);
+}
+
+/*****************************************************************************/
+
 static inline const char *
 nmtst_get_sudo_cmd (void)
 {
@@ -2323,13 +2430,4 @@ nmtst_keyfile_get_num_keys (GKeyFile *keyfile,
 
 /*****************************************************************************/
 
-static inline gboolean
-nmtst_g_source_assert_not_called (gpointer user_data)
-{
-	g_assert_not_reached ();
-	return G_SOURCE_CONTINUE;
-}
-
-/*****************************************************************************/
-
 #endif /* __NM_TEST_UTILS_H__ */