bluez: make connect operation (partially) cancellable and drop GAsyncResult pattern

All operations must be cancellable in a timely manner, otherwise, the objects
hang during shutdown.

Also, get rid of the GAsyncResult pattern. It is more cumbersome than
helpful.

There are still several issues, marked by FIXME comments.

5 files changed, 155 insertions, 115 deletions

diff --git a/src/devices/bluetooth/nm-bluez-device.c b/src/devices/bluetooth/nm-bluez-device.c
index b722f692ce..8e2a96b78f 100644
--- a/src/devices/bluetooth/nm-bluez-device.c
+++ b/src/devices/bluetooth/nm-bluez-device.c
@@ -451,6 +451,9 @@ nm_bluez_device_disconnect (NMBluezDevice *self)
 
 	g_return_if_fail (priv->dbus_connection);
 
+	/* FIXME: if we are in the process of connecting and cancel the
+	 * connection attempt, we must complete the pending connect request.
+	 * However, we must also ensure that we don't leave a connected device. */
 	if (priv->connection_bt_type == NM_BT_CAPABILITY_DUN) {
 		if (priv->bluez_version == 4) {
 			/* Can't pass a NULL interface name through dbus to bluez, so just
@@ -496,76 +499,109 @@ out:
 }
 
 static void
-bluez_connect_cb (GDBusConnection *dbus_connection,
+_connect_complete (NMBluezDevice *self,
+                   const char *device,
+                   NMBluezDeviceConnectCallback callback,
+                   gpointer callback_user_data,
+                   GError *error)
+{
+	NMBluezDevicePrivate *priv = NM_BLUEZ_DEVICE_GET_PRIVATE (self);
+
+	nm_assert ((device || error) && !(device && error));
+
+	if (   device
+	    && priv->bluez_version == 5) {
+		priv->connected = TRUE;
+		_notify (self, PROP_CONNECTED);
+	}
+
+	if (callback)
+		callback (self, device, error, callback_user_data);
+}
+
+static void
+_connect_cb (GObject *source_object,
                   GAsyncResult *res,
                   gpointer user_data)
 {
-	GSimpleAsyncResult *result = G_SIMPLE_ASYNC_RESULT (user_data);
-	GObject *result_object = g_async_result_get_source_object (G_ASYNC_RESULT (result));
-	NMBluezDevice *self = NM_BLUEZ_DEVICE (result_object);
-	NMBluezDevicePrivate *priv = NM_BLUEZ_DEVICE_GET_PRIVATE (self);
-	GError *error = NULL;
-	char *device;
-	GVariant *variant;
+	gs_unref_object NMBluezDevice *self = NULL;
+	NMBluezDevicePrivate *priv;
+	NMBluezDeviceConnectCallback callback;
+	gpointer callback_user_data;
+	gs_free_error GError *error = NULL;
+	char *device = NULL;
+	gs_unref_variant GVariant *variant = NULL;
 
-	variant = g_dbus_connection_call_finish (dbus_connection, res, &error);
+	nm_utils_user_data_unpack (user_data, &self, &callback, &callback_user_data);
 
-	if (!variant) {
-		g_simple_async_result_take_error (result, error);
-	} else {
-		g_variant_get (variant, "(s)", &device);
+	priv = NM_BLUEZ_DEVICE_GET_PRIVATE (self);
 
-		g_simple_async_result_set_op_res_gpointer (result,
-		                                           g_strdup (device),
-		                                           g_free);
+	variant = _nm_dbus_connection_call_finish (G_DBUS_CONNECTION (source_object), res, G_VARIANT_TYPE ("(s)"), &error);
+	if (variant) {
+		g_variant_get (variant, "(s)", &device);
 		priv->b4_iface = device;
-		g_variant_unref (variant);
 	}
 
-	g_simple_async_result_complete (result);
-	g_object_unref (result);
-	g_object_unref (result_object);
+	_connect_complete (self, device, callback, callback_user_data, error);
 }
 
 #if WITH_BLUEZ5_DUN
 static void
-bluez5_dun_connect_cb (NMBluez5DunContext *context,
-                   const char *device,
-                   GError *error,
-                   gpointer user_data)
+_connect_cb_bluez5_dun (NMBluez5DunContext *context,
+                        const char *device,
+                        GError *error,
+                        gpointer user_data)
 {
-	GSimpleAsyncResult *result = G_SIMPLE_ASYNC_RESULT (user_data);
+	gs_unref_object NMBluezDevice *self = NULL;
+	gs_unref_object GCancellable *cancellable = NULL;
+	NMBluezDeviceConnectCallback callback;
+	gpointer callback_user_data;
+	gs_free_error GError *cancelled_error = NULL;
 
-	if (error) {
-		g_simple_async_result_take_error (result, error);
-	} else {
-		g_simple_async_result_set_op_res_gpointer (result,
-		                                           g_strdup (device),
-		                                           g_free);
-	}
+	nm_utils_user_data_unpack (user_data, &self, &cancellable, &callback, &callback_user_data);
+
+	/* FIXME(shutdown): the async operation nm_bluez5_dun_connect() should be cancellable.
+	 * Fake it here. */
+	if (g_cancellable_set_error_if_cancelled (cancellable, &cancelled_error))
+		error = cancelled_error;
 
-	g_simple_async_result_complete (result);
-	g_object_unref (result);
+	_connect_complete (self, device, callback, callback_user_data, error);
 }
-#endif
+#else /* WITH_BLUEZ5_DUN */
+static void
+_connect_cb_bluez5_dun_idle_no_b5 (gpointer user_data,
+                                   GCancellable *cancellable)
+{
+	gs_unref_object NMBluezDevice *self = NULL;
+	NMBluezDeviceConnectCallback callback;
+	gpointer callback_user_data;
+	gs_free_error GError *error = NULL;
+
+	nm_utils_user_data_unpack (user_data, &self, &callback, &callback_user_data);
+
+	if (!g_cancellable_set_error_if_cancelled (cancellable, &error)) {
+		g_set_error (&error,
+		             NM_BT_ERROR,
+		             NM_BT_ERROR_DUN_CONNECT_FAILED,
+		             "NetworkManager built without support for Bluez 5");
+	}
+	callback (self, NULL, error, callback_user_data);
+}
+#endif /* WITH_BLUEZ5_DUN */
 
 void
 nm_bluez_device_connect_async (NMBluezDevice *self,
                                NMBluetoothCapabilities connection_bt_type,
-                               GAsyncReadyCallback callback,
-                               gpointer user_data)
+                               GCancellable *cancellable,
+                               NMBluezDeviceConnectCallback callback,
+                               gpointer callback_user_data)
 {
-	GSimpleAsyncResult *simple;
 	NMBluezDevicePrivate *priv = NM_BLUEZ_DEVICE_GET_PRIVATE (self);
 	const char *dbus_iface = NULL;
 	const char *connect_type = NULL;
 
 	g_return_if_fail (priv->capabilities & connection_bt_type & (NM_BT_CAPABILITY_DUN | NM_BT_CAPABILITY_NAP));
 
-	simple = g_simple_async_result_new (G_OBJECT (self),
-	                                    callback,
-	                                    user_data,
-	                                    nm_bluez_device_connect_async);
 	priv->connection_bt_type = connection_bt_type;
 
 	if (connection_bt_type == NM_BT_CAPABILITY_NAP) {
@@ -582,19 +618,29 @@ nm_bluez_device_connect_async (NMBluezDevice *self,
 #if WITH_BLUEZ5_DUN
 			if (priv->b5_dun_context == NULL)
 				priv->b5_dun_context = nm_bluez5_dun_new (priv->adapter_address, priv->address);
-			nm_bluez5_dun_connect (priv->b5_dun_context, bluez5_dun_connect_cb, simple);
+			nm_bluez5_dun_connect (priv->b5_dun_context,
+			                       _connect_cb_bluez5_dun,
+			                       nm_utils_user_data_pack (g_object_ref (self),
+			                                                nm_g_object_ref (cancellable),
+			                                                callback,
+			                                                callback_user_data));
 #else
-			g_simple_async_result_set_error (simple,
-							 NM_BT_ERROR,
-							 NM_BT_ERROR_DUN_CONNECT_FAILED,
-							 "NetworkManager built without support for Bluez 5");
-			g_simple_async_result_complete (simple);
+			if (callback) {
+				nm_utils_invoke_on_idle (_connect_cb_bluez5_dun_idle_no_b5,
+				                         nm_utils_user_data_pack (g_object_ref (self),
+				                                                  callback,
+				                                                  callback_user_data),
+				                         cancellable);
+			}
 #endif
 			return;
 		}
 	} else
-		g_assert_not_reached ();
+		g_return_if_reached ();
 
+	/* FIXME: we need to remember that a connect is in progress.
+	 * So, if the request gets cancelled, that we disconnect the
+	 * connection that was established in the meantime. */
 	g_dbus_connection_call (priv->dbus_connection,
 	                        NM_BLUEZ_SERVICE,
 	                        priv->path,
@@ -604,37 +650,11 @@ nm_bluez_device_connect_async (NMBluezDevice *self,
 	                        NULL,
 	                        G_DBUS_CALL_FLAGS_NONE,
 	                        20000,
-	                        NULL,
-	                        (GAsyncReadyCallback) bluez_connect_cb,
-	                        simple);
-}
-
-const char *
-nm_bluez_device_connect_finish (NMBluezDevice *self,
-                                GAsyncResult *result,
-                                GError **error)
-{
-	NMBluezDevicePrivate *priv = NM_BLUEZ_DEVICE_GET_PRIVATE (self);
-	GSimpleAsyncResult *simple;
-	const char *device;
-
-	g_return_val_if_fail (g_simple_async_result_is_valid (result,
-	                                                      G_OBJECT (self),
-	                                                      nm_bluez_device_connect_async),
-	                      NULL);
-
-	simple = (GSimpleAsyncResult *) result;
-
-	if (g_simple_async_result_propagate_error (simple, error))
-		return NULL;
-
-	device = (const char *) g_simple_async_result_get_op_res_gpointer (simple);
-	if (device && priv->bluez_version == 5) {
-		priv->connected = TRUE;
-		_notify (self, PROP_CONNECTED);
-	}
-
-	return device;
+	                        cancellable,
+	                        _connect_cb,
+	                        nm_utils_user_data_pack (g_object_ref (self),
+	                                                 callback,
+	                                                 callback_user_data));
 }
 
 /*****************************************************************************/
diff --git a/src/devices/bluetooth/nm-bluez-device.h b/src/devices/bluetooth/nm-bluez-device.h
index f8a1872f2e..d2d0beb016 100644
--- a/src/devices/bluetooth/nm-bluez-device.h
+++ b/src/devices/bluetooth/nm-bluez-device.h
@@ -66,16 +66,17 @@ guint32 nm_bluez_device_get_capabilities (NMBluezDevice *self);
 
 gboolean nm_bluez_device_get_connected (NMBluezDevice *self);
 
+typedef void (*NMBluezDeviceConnectCallback) (NMBluezDevice *self,
+                                              const char *device,
+                                              GError *error,
+                                              gpointer user_data);
+
 void
 nm_bluez_device_connect_async (NMBluezDevice *self,
                                NMBluetoothCapabilities connection_bt_type,
-                               GAsyncReadyCallback callback,
-                               gpointer user_data);
-
-const char *
-nm_bluez_device_connect_finish (NMBluezDevice *self,
-                                GAsyncResult *result,
-                                GError **error);
+                               GCancellable *cancellable,
+                               NMBluezDeviceConnectCallback callback,
+                               gpointer callback_user_data);
 
 void
 nm_bluez_device_disconnect (NMBluezDevice *self);
diff --git a/src/devices/bluetooth/nm-bluez5-dun.c b/src/devices/bluetooth/nm-bluez5-dun.c
index ca09b276e7..32f8da65eb 100644
--- a/src/devices/bluetooth/nm-bluez5-dun.c
+++ b/src/devices/bluetooth/nm-bluez5-dun.c
@@ -342,13 +342,14 @@ nm_bluez5_dun_connect (NMBluez5DunContext *context,
 
 	if (context->rfcomm_channel != -1) {
 		nm_log_dbg (LOGD_BT, "(%s): channel number on device %s cached: %d",
-			    context->src_str, context->dst_str, context->rfcomm_channel);
+		            context->src_str, context->dst_str, context->rfcomm_channel);
+		/* FIXME: don't invoke the callback synchronously. */
 		dun_connect (context);
 		return;
 	}
 
 	nm_log_dbg (LOGD_BT, "(%s): starting channel number discovery for device %s",
-		    context->src_str, context->dst_str);
+	            context->src_str, context->dst_str);
 
 	context->sdp_session = sdp_connect (&context->src, &context->dst, SDP_NON_BLOCKING);
 	if (!context->sdp_session) {
@@ -358,10 +359,12 @@ nm_bluez5_dun_connect (NMBluez5DunContext *context,
 		error = g_error_new (NM_BT_ERROR, NM_BT_ERROR_DUN_CONNECT_FAILED,
 		                     "Failed to connect to the SDP server: (%d) %s",
 		                      err, strerror (err));
+		/* FIXME: don't invoke the callback synchronously. */
 		context->callback (context, NULL, error, context->user_data);
 		return;
 	}
 
+	/* FIXME(shutdown): make connect cancellable. */
 	channel = g_io_channel_unix_new (sdp_get_socket (context->sdp_session));
 	context->sdp_watch_id = g_io_add_watch (channel,
 	                                        G_IO_OUT | G_IO_HUP | G_IO_ERR | G_IO_NVAL,
diff --git a/src/devices/bluetooth/nm-bluez5-dun.h b/src/devices/bluetooth/nm-bluez5-dun.h
index 124c1a05b0..b75e4399a7 100644
--- a/src/devices/bluetooth/nm-bluez5-dun.h
+++ b/src/devices/bluetooth/nm-bluez5-dun.h
@@ -32,7 +32,8 @@ NMBluez5DunContext *nm_bluez5_dun_new (const char *adapter,
                                        const char *remote);
 
 void nm_bluez5_dun_connect (NMBluez5DunContext *context,
-                            NMBluez5DunFunc callback, gpointer user_data);
+                            NMBluez5DunFunc callback,
+                            gpointer user_data);
 
 /* Clean up connection resources */
 void nm_bluez5_dun_cleanup (NMBluez5DunContext *context);
diff --git a/src/devices/bluetooth/nm-device-bt.c b/src/devices/bluetooth/nm-device-bt.c
index 1f650d339f..b05c569beb 100644
--- a/src/devices/bluetooth/nm-device-bt.c
+++ b/src/devices/bluetooth/nm-device-bt.c
@@ -78,7 +78,9 @@ typedef struct {
 
 	char *rfcomm_iface;
 	NMModem *modem;
-	guint32 timeout_id;
+	guint timeout_id;
+
+	GCancellable *cancellable;
 
 	guint32 bt_type;  /* BT type of the current connection */
 } NMDeviceBtPrivate;
@@ -672,6 +674,7 @@ component_added (NMDevice *device, GObject *component)
 
 	/* Got the modem */
 	nm_clear_g_source (&priv->timeout_id);
+	nm_clear_g_cancellable (&priv->cancellable);
 
 	/* Can only accept the modem in stage2, but since the interface matched
 	 * what we were expecting, don't let anything else claim the modem either.
@@ -715,8 +718,11 @@ static gboolean
 modem_find_timeout (gpointer user_data)
 {
 	NMDeviceBt *self = NM_DEVICE_BT (user_data);
+	NMDeviceBtPrivate *priv = NM_DEVICE_BT_GET_PRIVATE (self);
+
+	priv->timeout_id = 0;
+	nm_clear_g_cancellable (&priv->cancellable);
 
-	NM_DEVICE_BT_GET_PRIVATE (self)->timeout_id = 0;
 	nm_device_state_changed (NM_DEVICE (self),
 	                         NM_DEVICE_STATE_FAILED,
 	                         NM_DEVICE_STATE_REASON_MODEM_NOT_FOUND);
@@ -738,8 +744,8 @@ check_connect_continue (NMDeviceBt *self)
 	       "Activation: (bluetooth) Stage 2 of 5 (Device Configure) successful. Will connect via %s.",
 	       dun ? "DUN" : (pan ? "PAN" : "unknown"));
 
-	/* Kill the connect timeout since we're connected now */
 	nm_clear_g_source (&priv->timeout_id);
+	nm_clear_g_cancellable (&priv->cancellable);
 
 	if (pan) {
 		/* Bluez says we're connected now.  Start IP config. */
@@ -755,25 +761,25 @@ check_connect_continue (NMDeviceBt *self)
 }
 
 static void
-bluez_connect_cb (GObject *object,
-                  GAsyncResult *res,
-                  void *user_data)
+bluez_connect_cb (NMBluezDevice *bt_device,
+                  const char *device_name,
+                  GError *error,
+                  gpointer user_data)
 {
-	gs_unref_object NMDeviceBt *self = NM_DEVICE_BT (user_data);
+	gs_unref_object NMDeviceBt *self = user_data;
 	NMDeviceBtPrivate *priv = NM_DEVICE_BT_GET_PRIVATE (self);
-	GError *error = NULL;
-	const char *device;
 
-	device = nm_bluez_device_connect_finish (NM_BLUEZ_DEVICE (object),
-	                                         res, &error);
+	if (nm_utils_error_is_cancelled (error, FALSE))
+		return;
+
+	nm_clear_g_source (&priv->timeout_id);
+	g_clear_object (&priv->cancellable);
 
 	if (!nm_device_is_activating (NM_DEVICE (self)))
 		return;
 
-	if (!device) {
+	if (!device_name) {
 		_LOGW (LOGD_BT, "Error connecting with bluez: %s", error->message);
-		g_clear_error (&error);
-
 		nm_device_state_changed (NM_DEVICE (self),
 		                         NM_DEVICE_STATE_FAILED,
 		                         NM_DEVICE_STATE_REASON_BT_FAILED);
@@ -782,10 +788,10 @@ bluez_connect_cb (GObject *object,
 
 	if (priv->bt_type == NM_BT_CAPABILITY_DUN) {
 		g_free (priv->rfcomm_iface);
-		priv->rfcomm_iface = g_strdup (device);
+		priv->rfcomm_iface = g_strdup (device_name);
 	} else if (priv->bt_type == NM_BT_CAPABILITY_NAP) {
-		if (!nm_device_set_ip_iface (NM_DEVICE (self), device)) {
-			_LOGW (LOGD_BT, "Error connecting with bluez: cannot find device %s", device);
+		if (!nm_device_set_ip_iface (NM_DEVICE (self), device_name)) {
+			_LOGW (LOGD_BT, "Error connecting with bluez: cannot find device %s", device_name);
 			nm_device_state_changed (NM_DEVICE (self),
 			                         NM_DEVICE_STATE_FAILED,
 			                         NM_DEVICE_STATE_REASON_BT_FAILED);
@@ -843,10 +849,13 @@ static gboolean
 bt_connect_timeout (gpointer user_data)
 {
 	NMDeviceBt *self = NM_DEVICE_BT (user_data);
+	NMDeviceBtPrivate *priv = NM_DEVICE_BT_GET_PRIVATE (self);
 
 	_LOGD (LOGD_BT, "initial connection timed out");
 
-	NM_DEVICE_BT_GET_PRIVATE (self)->timeout_id = 0;
+	priv->timeout_id = 0;
+	nm_clear_g_cancellable (&priv->cancellable);
+
 	nm_device_state_changed (NM_DEVICE (self),
 	                         NM_DEVICE_STATE_FAILED,
 	                         NM_DEVICE_STATE_REASON_BT_FAILED);
@@ -876,13 +885,17 @@ act_stage2_config (NMDevice *device, NMDeviceStateReason *out_failure_reason)
 
 	_LOGD (LOGD_BT, "requesting connection to the device");
 
-	/* Connect to the BT device */
-	nm_bluez_device_connect_async (priv->bt_device,
-	                               priv->bt_type & (NM_BT_CAPABILITY_DUN | NM_BT_CAPABILITY_NAP),
-	                               bluez_connect_cb, g_object_ref (device));
-
 	nm_clear_g_source (&priv->timeout_id);
+	nm_clear_g_cancellable (&priv->cancellable);
+
 	priv->timeout_id = g_timeout_add_seconds (30, bt_connect_timeout, device);
+	priv->cancellable = g_cancellable_new ();
+
+	nm_bluez_device_connect_async (priv->bt_device,
+	                               priv->bt_type & (NM_BT_CAPABILITY_DUN | NM_BT_CAPABILITY_NAP),
+	                               priv->cancellable,
+	                               bluez_connect_cb,
+	                               g_object_ref (self));
 
 	return NM_ACT_STAGE_RETURN_POSTPONE;
 }
@@ -925,6 +938,9 @@ deactivate (NMDevice *device)
 	priv->have_iface = FALSE;
 	priv->connected = FALSE;
 
+	nm_clear_g_source (&priv->timeout_id);
+	nm_clear_g_cancellable (&priv->cancellable);
+
 	if (priv->bt_type == NM_BT_CAPABILITY_DUN) {
 		if (priv->modem) {
 			nm_modem_deactivate (priv->modem, device);
@@ -942,8 +958,6 @@ deactivate (NMDevice *device)
 	if (priv->bt_type != NM_BT_CAPABILITY_NONE)
 		nm_bluez_device_disconnect (priv->bt_device);
 
-	nm_clear_g_source (&priv->timeout_id);
-
 	priv->bt_type = NM_BT_CAPABILITY_NONE;
 
 	g_free (priv->rfcomm_iface);
@@ -1128,6 +1142,7 @@ dispose (GObject *object)
 	NMDeviceBtPrivate *priv = NM_DEVICE_BT_GET_PRIVATE ((NMDeviceBt *) object);
 
 	nm_clear_g_source (&priv->timeout_id);
+	nm_clear_g_cancellable (&priv->cancellable);
 
 	g_signal_handlers_disconnect_matched (priv->bt_device, G_SIGNAL_MATCH_DATA, 0, 0, NULL, NULL, object);