diff options
author | Thomas Haller <thaller@redhat.com> | 2018-10-11 10:54:17 +0200 |
---|---|---|
committer | Thomas Haller <thaller@redhat.com> | 2018-10-13 17:32:11 +0200 |
commit | d5f62d918eade847690ee252d20bf160c9a5a25b (patch) | |
tree | afe5b5f659a033a3db0335bacbcaeed3f0732d3c | |
parent | 7d49c9cd39bddb913a85fa94f8d10a3b077383b5 (diff) | |
download | NetworkManager-d5f62d918eade847690ee252d20bf160c9a5a25b.tar.gz |
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.
-rw-r--r-- | src/devices/bluetooth/nm-bluez-device.c | 180 | ||||
-rw-r--r-- | src/devices/bluetooth/nm-bluez-device.h | 15 | ||||
-rw-r--r-- | src/devices/bluetooth/nm-bluez5-dun.c | 7 | ||||
-rw-r--r-- | src/devices/bluetooth/nm-bluez5-dun.h | 3 | ||||
-rw-r--r-- | src/devices/bluetooth/nm-device-bt.c | 65 |
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); |