device: Always use an internal cancellable for tasks

This will allow libfprint to cancel operations internally in the future.

If the internal cancellation method is used, then the private
current_cancellation_reason variable must be set to the GError. This
error will be returned when set.

libfprint/fp-device-private.h

@@ -67,8 +67,11 @@ typedef struct
   /* We always make sure that only one task is run at a time. */
   FpiDeviceAction     current_action;
   GTask              *current_task;
+  GError             *current_cancellation_reason;
   GAsyncReadyCallback current_user_cb;
+  GCancellable       *current_cancellable;
   gulong              current_cancellable_id;
+  gulong              current_task_cancellable_id;
   GSource            *current_idle_cancel_source;
   GSource            *current_task_idle_return_source;
 

libfprint/fp-device.c

@@ -119,20 +119,40 @@ fp_device_cancelled_cb (GCancellable *cancellable, FpDevice *self)
   g_source_unref (priv->current_idle_cancel_source);
 }
 
+/* Forward the external task cancellable to the internal one. */
 static void
-maybe_cancel_on_cancelled (FpDevice     *device,
+fp_device_task_cancelled_cb (GCancellable *cancellable, FpDevice *self)
-                           GCancellable *cancellable)
+{
+  FpDevicePrivate *priv = fp_device_get_instance_private (self);
+
+  g_cancellable_cancel (priv->current_cancellable);
+}
+
+static void
+setup_task_cancellable (FpDevice *device)
 {
-  FpDeviceClass *cls = FP_DEVICE_GET_CLASS (device);
   FpDevicePrivate *priv = fp_device_get_instance_private (device);
+  FpDeviceClass *cls = FP_DEVICE_GET_CLASS (device);
 
-  if (!cancellable || !cls->cancel)
+  /* Create an internal cancellable and hook it up. */
-    return;
+  priv->current_cancellable = g_cancellable_new ();
+  if (cls->cancel)
+    {
+      priv->current_cancellable_id = g_cancellable_connect (priv->current_cancellable,
+                                                            G_CALLBACK (fp_device_cancelled_cb),
+                                                            device,
+                                                            NULL);
+    }
 
-  priv->current_cancellable_id = g_cancellable_connect (cancellable,
+  /* Task cancellable is the externally visible one, make our internal one
-                                                        G_CALLBACK (fp_device_cancelled_cb),
+   * a slave of the external one. */
-                                                        device,
+  if (g_task_get_cancellable (priv->current_task))
-                                                        NULL);
+    {
+      priv->current_task_cancellable_id = g_cancellable_connect (g_task_get_cancellable (priv->current_task),
+                                                                 G_CALLBACK (fp_device_task_cancelled_cb),
+                                                                 device,
+                                                                 NULL);
+    }
 }
 
 static void
@@ -343,7 +363,7 @@ fp_device_async_initable_init_async (GAsyncInitable     *initable,
 
   priv->current_action = FPI_DEVICE_ACTION_PROBE;
   priv->current_task = g_steal_pointer (&task);
-  maybe_cancel_on_cancelled (self, cancellable);
+  setup_task_cancellable (self);
 
   FP_DEVICE_GET_CLASS (self)->probe (self);
 }
@@ -800,7 +820,7 @@ fp_device_open (FpDevice           *device,
 
   priv->current_action = FPI_DEVICE_ACTION_OPEN;
   priv->current_task = g_steal_pointer (&task);
-  maybe_cancel_on_cancelled (device, cancellable);
+  setup_task_cancellable (device);
   fpi_device_report_finger_status (device, FP_FINGER_STATUS_NONE);
 
   FP_DEVICE_GET_CLASS (device)->open (device);
@@ -865,7 +885,7 @@ fp_device_close (FpDevice           *device,
 
   priv->current_action = FPI_DEVICE_ACTION_CLOSE;
   priv->current_task = g_steal_pointer (&task);
-  maybe_cancel_on_cancelled (device, cancellable);
+  setup_task_cancellable (device);
 
   FP_DEVICE_GET_CLASS (device)->close (device);
 }
@@ -963,7 +983,7 @@ fp_device_enroll (FpDevice           *device,
 
   priv->current_action = FPI_DEVICE_ACTION_ENROLL;
   priv->current_task = g_steal_pointer (&task);
-  maybe_cancel_on_cancelled (device, cancellable);
+  setup_task_cancellable (device);
 
   fpi_device_update_temp (device, TRUE);
 
@@ -1058,7 +1078,7 @@ fp_device_verify (FpDevice           *device,
 
   priv->current_action = FPI_DEVICE_ACTION_VERIFY;
   priv->current_task = g_steal_pointer (&task);
-  maybe_cancel_on_cancelled (device, cancellable);
+  setup_task_cancellable (device);
 
   fpi_device_update_temp (device, TRUE);
 
@@ -1179,7 +1199,7 @@ fp_device_identify (FpDevice           *device,
 
   priv->current_action = FPI_DEVICE_ACTION_IDENTIFY;
   priv->current_task = g_steal_pointer (&task);
-  maybe_cancel_on_cancelled (device, cancellable);
+  setup_task_cancellable (device);
 
   fpi_device_update_temp (device, TRUE);
 
@@ -1298,7 +1318,7 @@ fp_device_capture (FpDevice           *device,
 
   priv->current_action = FPI_DEVICE_ACTION_CAPTURE;
   priv->current_task = g_steal_pointer (&task);
-  maybe_cancel_on_cancelled (device, cancellable);
+  setup_task_cancellable (device);
 
   fpi_device_update_temp (device, TRUE);
 
@@ -1382,7 +1402,7 @@ fp_device_delete_print (FpDevice           *device,
 
   priv->current_action = FPI_DEVICE_ACTION_DELETE;
   priv->current_task = g_steal_pointer (&task);
-  maybe_cancel_on_cancelled (device, cancellable);
+  setup_task_cancellable (device);
 
   g_task_set_task_data (priv->current_task,
                         g_object_ref (enrolled_print),
@@ -1461,7 +1481,7 @@ fp_device_list_prints (FpDevice           *device,
 
   priv->current_action = FPI_DEVICE_ACTION_LIST;
   priv->current_task = g_steal_pointer (&task);
-  maybe_cancel_on_cancelled (device, cancellable);
+  setup_task_cancellable (device);
 
   cls->list (device);
 }
@@ -1546,7 +1566,7 @@ fp_device_clear_storage (FpDevice           *device,
 
   priv->current_action = FPI_DEVICE_ACTION_CLEAR_STORAGE;
   priv->current_task = g_steal_pointer (&task);
-  maybe_cancel_on_cancelled (device, cancellable);
+  setup_task_cancellable (device);
 
   cls->clear_storage (device);
 

libfprint/fpi-device.c

@@ -490,14 +490,11 @@ gboolean
 fpi_device_action_is_cancelled (FpDevice *device)
 {
   FpDevicePrivate *priv = fp_device_get_instance_private (device);
-  GCancellable *cancellable;
 
   g_return_val_if_fail (FP_IS_DEVICE (device), TRUE);
   g_return_val_if_fail (priv->current_action != FPI_DEVICE_ACTION_NONE, TRUE);
 
-  cancellable = g_task_get_cancellable (priv->current_task);
+  return g_cancellable_is_cancelled (priv->current_cancellable);
-
-  return g_cancellable_is_cancelled (cancellable);
 }
 
 /**
@@ -675,7 +672,7 @@ fpi_device_get_cancellable (FpDevice *device)
   g_return_val_if_fail (FP_IS_DEVICE (device), NULL);
   g_return_val_if_fail (priv->current_action != FPI_DEVICE_ACTION_NONE, NULL);
 
-  return g_task_get_cancellable (priv->current_task);
+  return priv->current_cancellable;
 }
 
 static void
@@ -813,10 +810,17 @@ clear_device_cancel_action (FpDevice *device)
 
   if (priv->current_cancellable_id)
     {
-      g_cancellable_disconnect (g_task_get_cancellable (priv->current_task),
+      g_cancellable_disconnect (priv->current_cancellable,
                                 priv->current_cancellable_id);
       priv->current_cancellable_id = 0;
     }
+
+  if (priv->current_task_cancellable_id)
+    {
+      g_cancellable_disconnect (g_task_get_cancellable (priv->current_task),
+                                priv->current_task_cancellable_id);
+      priv->current_task_cancellable_id = 0;
+    }
 }
 
 typedef enum _FpDeviceTaskReturnType {
@@ -843,6 +847,7 @@ fp_device_task_return_in_idle_cb (gpointer user_data)
   FpiDeviceAction action;
 
   g_autoptr(GTask) task = NULL;
+  g_autoptr(GError) cancellation_reason = NULL;
 
 
   action_str = g_enum_to_string (FPI_TYPE_DEVICE_ACTION, priv->current_action);
@@ -852,6 +857,8 @@ fp_device_task_return_in_idle_cb (gpointer user_data)
   action = priv->current_action;
   priv->current_action = FPI_DEVICE_ACTION_NONE;
   priv->current_task_idle_return_source = NULL;
+  g_clear_object (&priv->current_cancellable);
+  cancellation_reason = g_steal_pointer (&priv->current_cancellation_reason);
 
   fpi_device_update_temp (data->device, FALSE);
 
@@ -870,6 +877,8 @@ fp_device_task_return_in_idle_cb (gpointer user_data)
       g_object_notify (G_OBJECT (data->device), "open");
     }
 
+  /* TODO: Port/use the cancellation mechanism for device removal! */
+
   /* Return FP_DEVICE_ERROR_REMOVED if the device is removed,
    * with the exception of a successful open, which is an odd corner case. */
   if (priv->is_removed &&
@@ -884,6 +893,15 @@ fp_device_task_return_in_idle_cb (gpointer user_data)
       return G_SOURCE_REMOVE;
     }
 
+  /* Return internal cancellation reason if we have one.
+   * Note that an external cancellation always returns G_IO_ERROR_CANCELLED */
+  if (cancellation_reason)
+    {
+      g_task_return_error (task, g_steal_pointer (&cancellation_reason));
+
+      return G_SOURCE_REMOVE;
+    }
+
   switch (data->type)
     {
     case FP_DEVICE_TASK_RETURN_INT:

tests/test-device-fake.h

@@ -32,6 +32,8 @@ struct _FpiDeviceFake
   gpointer        last_called_function;
   gboolean        return_action_error;
 
+  GCancellable   *ext_cancellable;
+
   GError         *ret_error;
   FpPrint        *ret_print;
   FpPrint        *ret_match;

tests/test-fpi-device.c

@@ -2356,32 +2356,32 @@ test_driver_action_get_cancellable_open (void)
 }
 
 static void
-test_driver_action_get_cancellable_open_fail_vfunc (FpDevice *device)
+test_driver_action_get_cancellable_open_internal_vfunc (FpDevice *device)
 {
   FpiDeviceFake *fake_dev = FPI_DEVICE_FAKE (device);
 
   g_assert_cmpuint (fpi_device_get_current_action (device), ==, FPI_DEVICE_ACTION_OPEN);
-  fake_dev->last_called_function = test_driver_action_get_cancellable_open_fail_vfunc;
+  fake_dev->last_called_function = test_driver_action_get_cancellable_open_internal_vfunc;
 
-  g_assert_false (G_IS_CANCELLABLE (fpi_device_get_cancellable (device)));
+  g_assert_true (G_IS_CANCELLABLE (fpi_device_get_cancellable (device)));
 
   fpi_device_open_complete (device, NULL);
 }
 
 static void
-test_driver_action_get_cancellable_open_fail (void)
+test_driver_action_get_cancellable_open_internal (void)
 {
   g_autoptr(FpAutoResetClass) dev_class = auto_reset_device_class ();
   g_autoptr(FpAutoCloseDevice) device = NULL;
   FpiDeviceFake *fake_dev;
 
-  dev_class->open = test_driver_action_get_cancellable_open_fail_vfunc;
+  dev_class->open = test_driver_action_get_cancellable_open_internal_vfunc;
   device = g_object_new (FPI_TYPE_DEVICE_FAKE, NULL);
   fake_dev = FPI_DEVICE_FAKE (device);
 
   g_assert_true (fp_device_open_sync (device, NULL, NULL));
 
-  g_assert (fake_dev->last_called_function == test_driver_action_get_cancellable_open_fail_vfunc);
+  g_assert (fake_dev->last_called_function == test_driver_action_get_cancellable_open_internal_vfunc);
 }
 
 static void
@@ -2406,7 +2406,11 @@ test_driver_action_is_cancelled_open_vfunc (FpDevice *device)
   g_assert_true (G_IS_CANCELLABLE (fpi_device_get_cancellable (device)));
   g_assert_false (fpi_device_action_is_cancelled (device));
 
-  g_cancellable_cancel (fpi_device_get_cancellable (device));
+  if (fake_dev->ext_cancellable)
+    g_cancellable_cancel (fake_dev->ext_cancellable);
+  else
+    g_cancellable_cancel (fpi_device_get_cancellable (device));
+
   g_assert_true (fpi_device_action_is_cancelled (device));
 
   fpi_device_open_complete (device, NULL);
@@ -2425,13 +2429,34 @@ test_driver_action_is_cancelled_open (void)
   device = g_object_new (FPI_TYPE_DEVICE_FAKE, NULL);
   fake_dev = FPI_DEVICE_FAKE (device);
 
-  cancellable = g_cancellable_new ();
+  cancellable = fake_dev->ext_cancellable = g_cancellable_new ();
   g_assert_false (fp_device_open_sync (device, cancellable, &error));
   g_assert_error (error, G_IO_ERROR, G_IO_ERROR_CANCELLED);
 
   g_assert (fake_dev->last_called_function == test_driver_action_is_cancelled_open_vfunc);
 }
 
+static void
+test_driver_action_internally_cancelled_open (void)
+{
+  g_autoptr(FpAutoResetClass) dev_class = auto_reset_device_class ();
+  g_autoptr(FpAutoCloseDevice) device = NULL;
+  g_autoptr(GCancellable) cancellable = NULL;
+  g_autoptr(GError) error = NULL;
+  FpiDeviceFake *fake_dev;
+
+  dev_class->open = test_driver_action_is_cancelled_open_vfunc;
+  device = g_object_new (FPI_TYPE_DEVICE_FAKE, NULL);
+  fake_dev = FPI_DEVICE_FAKE (device);
+
+  /* No error, just some internal cancellation but we let nothing happen externally. */
+  cancellable = g_cancellable_new ();
+  g_assert_true (fp_device_open_sync (device, cancellable, &error));
+  g_assert_null (error);
+
+  g_assert (fake_dev->last_called_function == test_driver_action_is_cancelled_open_vfunc);
+}
+
 static void
 test_driver_action_is_cancelled_error (void)
 {
@@ -2897,8 +2922,9 @@ main (int argc, char *argv[])
   g_test_add_func ("/driver/get_current_action/open", test_driver_current_action_open);
   g_test_add_func ("/driver/get_cancellable/error", test_driver_action_get_cancellable_error);
   g_test_add_func ("/driver/get_cancellable/open", test_driver_action_get_cancellable_open);
-  g_test_add_func ("/driver/get_cancellable/open/fail", test_driver_action_get_cancellable_open_fail);
+  g_test_add_func ("/driver/get_cancellable/open/internal", test_driver_action_get_cancellable_open_internal);
   g_test_add_func ("/driver/action_is_cancelled/open", test_driver_action_is_cancelled_open);
+  g_test_add_func ("/driver/action_is_cancelled/open/internal", test_driver_action_internally_cancelled_open);
   g_test_add_func ("/driver/action_is_cancelled/error", test_driver_action_is_cancelled_error);
   g_test_add_func ("/driver/complete_action/all/error", test_driver_complete_actions_errors);
   g_test_add_func ("/driver/action_error/error", test_driver_action_error_error);