diff --git a/libfprint/fp-context.c b/libfprint/fp-context.c index 70d4062a..08bfd4c7 100644 --- a/libfprint/fp-context.c +++ b/libfprint/fp-context.c @@ -48,8 +48,6 @@ typedef struct GUsbContext *usb_ctx; GCancellable *cancellable; - GSList *sources; - gint pending_devices; gboolean enumerated; @@ -89,76 +87,38 @@ is_driver_allowed (const gchar *driver) return g_strv_contains ((const gchar * const *) allowlisted_drivers, driver); } -typedef struct +static void +remove_device (FpContext *context, + FpDevice *device) { - FpContext *context; - FpDevice *device; - GSource *source; -} RemoveDeviceData; - -static gboolean -remove_device_idle_cb (RemoveDeviceData *data) -{ - FpContextPrivate *priv = fp_context_get_instance_private (data->context); + FpContextPrivate *priv = fp_context_get_instance_private (context); guint idx = 0; - g_return_val_if_fail (g_ptr_array_find (priv->devices, data->device, &idx), G_SOURCE_REMOVE); + g_return_if_fail (g_ptr_array_find (priv->devices, device, &idx)); - g_signal_emit (data->context, signals[DEVICE_REMOVED_SIGNAL], 0, data->device); + g_object_ref (device); g_ptr_array_remove_index_fast (priv->devices, idx); - - return G_SOURCE_REMOVE; -} - -static void -remove_device_data_free (RemoveDeviceData *data) -{ - FpContextPrivate *priv = fp_context_get_instance_private (data->context); - - priv->sources = g_slist_remove (priv->sources, data->source); - g_free (data); -} - -static void -remove_device (FpContext *context, FpDevice *device) -{ - g_autoptr(GSource) source = NULL; - FpContextPrivate *priv = fp_context_get_instance_private (context); - RemoveDeviceData *data; - - data = g_new (RemoveDeviceData, 1); - data->context = context; - data->device = device; - - source = data->source = g_idle_source_new (); - g_source_set_callback (source, - G_SOURCE_FUNC (remove_device_idle_cb), data, - (GDestroyNotify) remove_device_data_free); - g_source_attach (source, g_main_context_get_thread_default ()); - - priv->sources = g_slist_prepend (priv->sources, source); + g_signal_emit (context, signals[DEVICE_REMOVED_SIGNAL], 0, device); + g_clear_object (&device); } static void device_remove_on_notify_open_cb (FpContext *context, GParamSpec *pspec, FpDevice *device) { + g_signal_handlers_disconnect_by_func (device, device_remove_on_notify_open_cb, context); remove_device (context, device); } static void device_removed_cb (FpContext *context, FpDevice *device) { - gboolean open = FALSE; - - g_object_get (device, "open", &open, NULL); - /* Wait for device close if the device is currently still open. */ - if (open) + if (fp_device_is_open (device)) { g_signal_connect_object (device, "notify::open", (GCallback) device_remove_on_notify_open_cb, context, - G_CONNECT_SWAPPED); + G_CONNECT_SWAPPED | G_CONNECT_AFTER); } else { @@ -194,7 +154,7 @@ async_device_init_done_cb (GObject *source_object, GAsyncResult *res, gpointer u g_signal_connect_object (device, "removed", (GCallback) device_removed_cb, context, - G_CONNECT_SWAPPED); + G_CONNECT_SWAPPED | G_CONNECT_AFTER); g_signal_emit (context, signals[DEVICE_ADDED_SIGNAL], 0, device); } @@ -274,8 +234,10 @@ usb_device_removed_cb (FpContext *self, GUsbDevice *device, GUsbContext *usb_ctx if (cls->type != FP_DEVICE_TYPE_USB) continue; - if (fpi_device_get_usb_device (dev) == device) - fpi_device_remove (dev); + if (fpi_device_get_usb_device (dev) != device) + continue; + + fpi_device_remove (dev); } } @@ -290,8 +252,6 @@ fp_context_finalize (GObject *object) g_clear_pointer (&priv->drivers, g_array_unref); g_clear_pointer (&priv->devices, g_ptr_array_unref); - g_slist_free_full (g_steal_pointer (&priv->sources), (GDestroyNotify) g_source_destroy); - if (priv->usb_ctx) g_object_run_dispose (G_OBJECT (priv->usb_ctx)); g_clear_object (&priv->usb_ctx); @@ -581,7 +541,7 @@ fp_context_enumerate (FpContext *context) * * Get all devices. fp_context_enumerate() will be called as needed. * - * Returns: (transfer none) (element-type FpDevice): a new #GPtrArray of #FpDevice's. + * Returns: (transfer none) (element-type FpDevice): a #GPtrArray of #FpDevice's. */ GPtrArray * fp_context_get_devices (FpContext *context) diff --git a/tests/test-fp-context.c b/tests/test-fp-context.c index c74548cd..23695342 100644 --- a/tests/test-fp-context.c +++ b/tests/test-fp-context.c @@ -115,36 +115,28 @@ context_device_removed_cb (FpContext *ctx, FpDevice *device, FptContext *tctx) /* "device-removed" on context is always after "removed" on device */ g_assert_cmpint (GPOINTER_TO_INT (tctx->user_data), ==, DEV_REMOVED_CB); tctx->user_data = GINT_TO_POINTER (CTX_DEVICE_REMOVED_CB); + + g_assert_false (g_ptr_array_find (fp_context_get_devices (tctx->fp_context), device, NULL)); } static void test_context_remove_device_closed (void) { g_autoptr(FptContext) tctx = fpt_context_new_with_virtual_device (FPT_VIRTUAL_DEVICE_IMAGE); - gboolean removed; + FpDevice *device = tctx->device; tctx->user_data = NULL; - g_signal_connect (tctx->device, "removed", (GCallback) device_removed_cb, tctx); g_signal_connect (tctx->fp_context, "device-removed", (GCallback) context_device_removed_cb, tctx); + g_signal_connect (tctx->device, "removed", (GCallback) device_removed_cb, tctx); /* Triggering remove on closed device. */ fpi_device_remove (tctx->device); - g_assert_nonnull (tctx->device); - g_object_get (tctx->device, "removed", &removed, NULL); - g_assert_true (removed); - g_assert_cmpint (GPOINTER_TO_INT (tctx->user_data), ==, DEV_REMOVED_CB); - - /* device-removed is dispatched from idle. */ - while (g_main_context_iteration (NULL, FALSE)) - { - } - /* The device is now destroyed and device-removed was called. */ g_assert_null (tctx->device); g_assert_cmpint (GPOINTER_TO_INT (tctx->user_data), ==, CTX_DEVICE_REMOVED_CB); - fpt_teardown_virtual_device_environment (); + g_assert_false (g_ptr_array_find (fp_context_get_devices (tctx->fp_context), device, NULL)); } static void @@ -165,6 +157,7 @@ test_context_remove_device_closing (void) g_autoptr(FptContext) tctx = fpt_context_new_with_virtual_device (FPT_VIRTUAL_DEVICE_IMAGE); g_autoptr(GError) close_error = NULL; g_autoptr(GError) error = NULL; + FpDevice *device = tctx->device; gboolean removed; tctx->user_data = NULL; @@ -190,18 +183,10 @@ test_context_remove_device_closing (void) g_assert_error (close_error, FP_DEVICE_ERROR, FP_DEVICE_ERROR_REMOVED); /* Now the removed callback has been called already. */ - g_assert_nonnull (tctx->device); - g_assert_cmpint (GPOINTER_TO_INT (tctx->user_data), ==, DEV_REMOVED_CB); - - /* While device-removed needs another idle iteration. */ - while (g_main_context_iteration (NULL, FALSE)) - { - } - g_assert_null (tctx->device); g_assert_cmpint (GPOINTER_TO_INT (tctx->user_data), ==, CTX_DEVICE_REMOVED_CB); - fpt_teardown_virtual_device_environment (); + g_assert_false (g_ptr_array_find (fp_context_get_devices (tctx->fp_context), device, NULL)); } static void @@ -234,20 +219,11 @@ test_context_remove_device_open (void) } g_assert_cmpint (GPOINTER_TO_INT (tctx->user_data), ==, DEV_REMOVED_CB); - /* On close, the device will be removed from the context, - * but only a main loop iteration later. */ + /* On close, the device will be removed from the context */ fp_device_close_sync (tctx->device, NULL, &error); g_assert_error (error, FP_DEVICE_ERROR, FP_DEVICE_ERROR_REMOVED); - g_assert_nonnull (tctx->device); - g_assert_cmpint (GPOINTER_TO_INT (tctx->user_data), ==, DEV_REMOVED_CB); - - while (g_main_context_iteration (NULL, FALSE)) - { - } g_assert_null (tctx->device); g_assert_cmpint (GPOINTER_TO_INT (tctx->user_data), ==, CTX_DEVICE_REMOVED_CB); - - fpt_teardown_virtual_device_environment (); } static void @@ -298,18 +274,55 @@ test_context_remove_device_opening (void) fp_device_close_sync (tctx->device, NULL, &close_error); g_assert_error (close_error, FP_DEVICE_ERROR, FP_DEVICE_ERROR_REMOVED); - g_assert_nonnull (tctx->device); - g_assert_cmpint (GPOINTER_TO_INT (tctx->user_data), ==, DEV_REMOVED_CB); + g_assert_null (tctx->device); + g_assert_cmpint (GPOINTER_TO_INT (tctx->user_data), ==, CTX_DEVICE_REMOVED_CB); +} - /* The device-removed signal needs an idle iteration. */ - while (g_main_context_iteration (NULL, FALSE)) - { - } +static void +open_close_cb (GObject *device, GAsyncResult *res, gpointer user_data) +{ + g_autoptr(GError) error = NULL; + g_autoptr(FpPrint) print = NULL; + gboolean *data = user_data; + + g_assert_true (fp_device_open_finish (FP_DEVICE (device), res, &error)); + g_assert_null (print); + g_assert_null (error); + + g_assert_false (fp_device_close_sync (FP_DEVICE (device), NULL, &error)); + g_assert_error (error, FP_DEVICE_ERROR, FP_DEVICE_ERROR_REMOVED); + + *data = TRUE; +} + +static void +test_context_remove_device_closes_on_open_callback (void) +{ + g_autoptr(FptContext) tctx = fpt_context_new_with_virtual_device (FPT_VIRTUAL_DEVICE_IMAGE); + gboolean open_done = FALSE; + gboolean removed; + + tctx->user_data = NULL; + g_signal_connect (tctx->device, "removed", (GCallback) device_removed_cb, tctx); + g_signal_connect (tctx->fp_context, "device-removed", (GCallback) context_device_removed_cb, tctx); + + fp_device_open (tctx->device, NULL, open_close_cb, &open_done); + g_assert_false (open_done); + + fpi_device_remove (tctx->device); + + /* Removed but not yet notified*/ + g_assert_nonnull (tctx->device); + g_object_get (tctx->device, "removed", &removed, NULL); + g_assert_true (removed); + g_assert_null (tctx->user_data); + + /* Running the mainloop now will cause the open to *succeed* despite removal! */ + while (!open_done) + g_main_context_iteration (NULL, TRUE); g_assert_null (tctx->device); g_assert_cmpint (GPOINTER_TO_INT (tctx->user_data), ==, CTX_DEVICE_REMOVED_CB); - - fpt_teardown_virtual_device_environment (); } static void @@ -369,18 +382,42 @@ test_context_remove_device_active (void) /* Now we close the device, state remains unchanged mostly. */ fp_device_close_sync (tctx->device, NULL, &error); g_assert_error (error, FP_DEVICE_ERROR, FP_DEVICE_ERROR_REMOVED); - g_assert_nonnull (tctx->device); - g_assert_cmpint (GPOINTER_TO_INT (tctx->user_data), ==, DEV_REMOVED_CB); - - /* And "device-removed" is called */ - while (g_main_context_iteration (NULL, FALSE)) - { - } - g_assert_null (tctx->device); g_assert_cmpint (GPOINTER_TO_INT (tctx->user_data), ==, CTX_DEVICE_REMOVED_CB); +} - fpt_teardown_virtual_device_environment (); +static void +device_removed_close_cb (FpDevice *device, FptContext *tctx) +{ + g_autoptr(GError) error = NULL; + g_assert_nonnull (device); + g_assert_true (device == tctx->device); + + g_assert_null (tctx->user_data); + tctx->user_data = GINT_TO_POINTER (DEV_REMOVED_CB); + + g_assert_true (fp_device_is_open (tctx->device)); + fp_device_close_sync (tctx->device, NULL, &error); + g_assert_error (error, FP_DEVICE_ERROR, FP_DEVICE_ERROR_REMOVED); + g_assert_false (fp_device_is_open (tctx->device)); + + g_assert_cmpint (GPOINTER_TO_INT (tctx->user_data), ==, DEV_REMOVED_CB); +} + +static void +test_context_remove_device_close_on_removal (void) +{ + g_autoptr(FptContext) tctx = fpt_context_new_with_virtual_device (FPT_VIRTUAL_DEVICE_IMAGE); + g_autoptr(GError) error = NULL; + + fp_device_open_sync (tctx->device, NULL, &error); + g_assert_no_error (error); + + g_signal_connect (tctx->fp_context, "device-removed", (GCallback) context_device_removed_cb, tctx); + g_signal_connect (tctx->device, "removed", (GCallback) device_removed_close_cb, tctx); + + fpi_device_remove (tctx->device); + g_assert_cmpint (GPOINTER_TO_INT (tctx->user_data), ==, CTX_DEVICE_REMOVED_CB); } int @@ -397,6 +434,8 @@ main (int argc, char *argv[]) g_test_add_func ("/context/remove-device-open", test_context_remove_device_open); g_test_add_func ("/context/remove-device-opening", test_context_remove_device_opening); g_test_add_func ("/context/remove-device-active", test_context_remove_device_active); + g_test_add_func ("/context/remove-device-close-on-removal", test_context_remove_device_close_on_removal); + g_test_add_func ("/context/remove-device-closes-on-open-callback", test_context_remove_device_closes_on_open_callback); return g_test_run (); }