Rework device removal to have a nice API

This enhances the device removal to create a well defined behaviour.

Primarily, it means that:
 * "device-removed" will only be called for closed devices
 * "removed" will be called only when no operation is active

Note that all actions will fail with FP_DEVICE_ERROR_REMOVED, *except*
for open which will only return this error if it failed.

Resolves: #330

libfprint/fp-context.c

@@ -86,6 +86,60 @@ is_driver_allowed (const gchar *driver)
   return FALSE;
 }
 
+typedef struct
+{
+  FpContext *context;
+  FpDevice  *device;
+} RemoveDeviceData;
+
+static gboolean
+remove_device_idle_cb (RemoveDeviceData *data)
+{
+  FpContextPrivate *priv = fp_context_get_instance_private (data->context);
+  guint idx = 0;
+
+  g_return_val_if_fail (g_ptr_array_find (priv->devices, data->device, &idx), G_SOURCE_REMOVE);
+
+  g_signal_emit (data->context, signals[DEVICE_REMOVED_SIGNAL], 0, data->device);
+  g_ptr_array_remove_index_fast (priv->devices, idx);
+
+  g_free (data);
+
+  return G_SOURCE_REMOVE;
+}
+
+static void
+remove_device (FpContext *context, FpDevice *device)
+{
+  RemoveDeviceData *data;
+
+  data = g_new (RemoveDeviceData, 1);
+  data->context = context;
+  data->device = device;
+
+  g_idle_add ((GSourceFunc) remove_device_idle_cb, data);
+}
+
+static void
+device_remove_on_notify_open_cb (FpContext *context, GParamSpec *pspec, FpDevice *device)
+{
+  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)
+    g_signal_connect_swapped (device, "notify::open", (GCallback) device_remove_on_notify_open_cb, context);
+  else
+    remove_device (context, device);
+}
+
 static void
 async_device_init_done_cb (GObject *source_object, GAsyncResult *res, gpointer user_data)
 {
@@ -110,6 +164,9 @@ async_device_init_done_cb (GObject *source_object, GAsyncResult *res, gpointer u
     }
 
   g_ptr_array_add (priv->devices, device);
+
+  g_signal_connect_swapped (device, "removed", (GCallback) device_removed_cb, context);
+
   g_signal_emit (context, signals[DEVICE_ADDED_SIGNAL], 0, device);
 }
 
@@ -189,12 +246,7 @@ usb_device_removed_cb (FpContext *self, GUsbDevice *device, GUsbContext *usb_ctx
         continue;
 
       if (fpi_device_get_usb_device (dev) == device)
-        {
+        fpi_device_remove (dev);
-          g_signal_emit (self, signals[DEVICE_REMOVED_SIGNAL], 0, dev);
-          g_ptr_array_remove_index_fast (priv->devices, i);
-
-          return;
-        }
     }
 }
 
@@ -248,6 +300,10 @@ fp_context_class_init (FpContextClass *klass)
    * @device: A #FpDevice
    *
    * This signal is emitted when a fingerprint reader is removed.
+   *
+   * It is guaranteed that the device has been closed before this signal
+   * is emitted. See the #FpDevice removed signal documentation for more
+   * information.
    **/
   signals[DEVICE_REMOVED_SIGNAL] = g_signal_new ("device-removed",
                                                  G_TYPE_FROM_CLASS (klass),

libfprint/fp-device-private.h

@@ -29,6 +29,7 @@ typedef struct
   GUsbDevice  *usb_device;
   const gchar *virtual_env;
 
+  gboolean     is_removed;
   gboolean     is_open;
 
   gchar       *device_id;

libfprint/fp-device.c

@@ -44,6 +44,7 @@ enum {
   PROP_DEVICE_ID,
   PROP_NAME,
   PROP_OPEN,
+  PROP_REMOVED,
   PROP_NR_ENROLL_STAGES,
   PROP_SCAN_TYPE,
   PROP_FINGER_STATUS,
@@ -55,6 +56,13 @@ enum {
 
 static GParamSpec *properties[N_PROPS];
 
+enum {
+  REMOVED_SIGNAL,
+  N_SIGNALS
+};
+
+static guint signals[N_SIGNALS] = { 0, };
+
 /**
  * fp_device_retry_quark:
  *
@@ -204,6 +212,10 @@ fp_device_get_property (GObject    *object,
       g_value_set_boolean (value, priv->is_open);
       break;
 
+    case PROP_REMOVED:
+      g_value_set_boolean (value, priv->is_removed);
+      break;
+
     default:
       G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec);
     }
@@ -351,6 +363,41 @@ fp_device_class_init (FpDeviceClass *klass)
                           "Whether the device is open or not", FALSE,
                           G_PARAM_STATIC_STRINGS | G_PARAM_READABLE);
 
+  properties[PROP_REMOVED] =
+    g_param_spec_boolean ("removed",
+                          "Removed",
+                          "Whether the device has been removed from the system", FALSE,
+                          G_PARAM_STATIC_STRINGS | G_PARAM_READABLE);
+
+  /**
+   * FpDevice::removed:
+   * @device: the #FpDevice instance that emitted the signal
+   *
+   * This signal is emitted after the device has been removed and no operation
+   * is pending anymore.
+   *
+   * The API user is still required to close a removed device. The above
+   * guarantee means that the call to close the device can be made immediately
+   * from the signal handler.
+   *
+   * The close operation will return FP_DEVICE_ERROR_REMOVED, but the device
+   * will still be considered closed afterwards.
+   *
+   * The device will only be removed from the #FpContext after it has been
+   * closed by the API user.
+   **/
+  signals[REMOVED_SIGNAL] = g_signal_new ("removed",
+                                          G_TYPE_FROM_CLASS (klass),
+                                          G_SIGNAL_RUN_LAST,
+                                          0,
+                                          NULL,
+                                          NULL,
+                                          g_cclosure_marshal_VOID__VOID,
+                                          G_TYPE_NONE,
+                                          0);
+
+  /* Private properties */
+
   /**
    * FpDevice::fpi-environ: (skip)
    *

libfprint/fp-device.h

@@ -91,6 +91,7 @@ typedef enum {
  * @FP_DEVICE_ERROR_DATA_NOT_FOUND: Requested print was not found on device
  * @FP_DEVICE_ERROR_DATA_FULL: No space on device available for operation
  * @FP_DEVICE_ERROR_DATA_DUPLICATE: Enrolling template duplicates storaged templates
+ * @FP_DEVICE_ERROR_REMOVED: The device has been removed.
  *
  * Error codes for device operations. More specific errors from other domains
  * such as #G_IO_ERROR or #G_USB_DEVICE_ERROR may also be reported.
@@ -106,6 +107,8 @@ typedef enum {
   FP_DEVICE_ERROR_DATA_NOT_FOUND,
   FP_DEVICE_ERROR_DATA_FULL,
   FP_DEVICE_ERROR_DATA_DUPLICATE,
+  /* Leave some room to add more DATA related errors */
+  FP_DEVICE_ERROR_REMOVED = 0x100,
 } FpDeviceError;
 
 GQuark fp_device_retry_quark (void);

libfprint/fpi-device.c

@@ -139,6 +139,10 @@ fpi_device_error_new (FpDeviceError error)
       msg = "This finger has already enrolled, please try a different finger";
       break;
 
+    case FP_DEVICE_ERROR_REMOVED:
+      msg = "This device has been removed from the system.";
+      break;
+
     default:
       g_warning ("Unsupported error, returning general error instead!");
       error = FP_DEVICE_ERROR_GENERAL;
@@ -576,6 +580,49 @@ fpi_device_get_cancellable (FpDevice *device)
   return g_task_get_cancellable (priv->current_task);
 }
 
+static void
+emit_removed_on_task_completed (FpDevice *device)
+{
+  g_signal_emit_by_name (device, "removed");
+}
+
+/**
+ * fpi_device_remove:
+ * @device: The #FpDevice
+ *
+ * Called to signal to the #FpDevice that it has been unplugged (physically
+ * removed from the system).
+ *
+ * For USB devices, this API is called automatically by #FpContext.
+ */
+void
+fpi_device_remove (FpDevice *device)
+{
+  FpDevicePrivate *priv = fp_device_get_instance_private (device);
+
+  g_return_if_fail (FP_IS_DEVICE (device));
+  g_return_if_fail (!priv->is_removed);
+
+  priv->is_removed = TRUE;
+
+  g_object_notify (G_OBJECT (device), "removed");
+
+  /* If there is a pending action, we wait for it to fail, otherwise we
+   * immediately emit the "removed" signal. */
+  if (priv->current_task)
+    {
+      g_signal_connect_object (priv->current_task,
+                               "notify::completed",
+                               (GCallback) emit_removed_on_task_completed,
+                               device,
+                               G_CONNECT_SWAPPED);
+    }
+  else
+    {
+      g_signal_emit_by_name (device, "removed");
+    }
+}
+
 /**
  * fpi_device_action_error:
  * @device: The #FpDevice
@@ -691,6 +738,7 @@ fp_device_task_return_in_idle_cb (gpointer user_data)
   FpDeviceTaskReturnData *data = user_data;
   FpDevicePrivate *priv = fp_device_get_instance_private (data->device);
   g_autofree char *action_str = NULL;
+  FpiDeviceAction action;
 
   g_autoptr(GTask) task = NULL;
 
@@ -699,9 +747,24 @@ fp_device_task_return_in_idle_cb (gpointer user_data)
   g_debug ("Completing action %s in idle!", action_str);
 
   task = g_steal_pointer (&priv->current_task);
+  action = priv->current_action;
   priv->current_action = FPI_DEVICE_ACTION_NONE;
   priv->current_task_idle_return_source = NULL;
 
+  /* 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 &&
+      ((action != FPI_DEVICE_ACTION_OPEN) ||
+       (action == FPI_DEVICE_ACTION_OPEN && data->type == FP_DEVICE_TASK_RETURN_ERROR)))
+    {
+      g_task_return_error (task, fpi_device_error_new (FP_DEVICE_ERROR_REMOVED));
+
+      /* NOTE: The removed signal will be emitted from the GTask
+       *       notify::completed if that is neccessary. */
+
+      return G_SOURCE_REMOVE;
+    }
+
   switch (data->type)
     {
     case FP_DEVICE_TASK_RETURN_INT:
@@ -713,16 +776,17 @@ fp_device_task_return_in_idle_cb (gpointer user_data)
       break;
 
     case FP_DEVICE_TASK_RETURN_OBJECT:
-      g_task_return_pointer (task, data->result, g_object_unref);
+      g_task_return_pointer (task, g_steal_pointer (&data->result),
+                             g_object_unref);
       break;
 
     case FP_DEVICE_TASK_RETURN_PTR_ARRAY:
-      g_task_return_pointer (task, data->result,
+      g_task_return_pointer (task, g_steal_pointer (&data->result),
                              (GDestroyNotify) g_ptr_array_unref);
       break;
 
     case FP_DEVICE_TASK_RETURN_ERROR:
-      g_task_return_error (task, data->result);
+      g_task_return_error (task, g_steal_pointer (&data->result));
       break;
 
     default:
@@ -735,6 +799,30 @@ fp_device_task_return_in_idle_cb (gpointer user_data)
 static void
 fpi_device_task_return_data_free (FpDeviceTaskReturnData *data)
 {
+  if (data->result)
+    {
+      switch (data->type)
+        {
+        case FP_DEVICE_TASK_RETURN_INT:
+        case FP_DEVICE_TASK_RETURN_BOOL:
+          break;
+
+        case FP_DEVICE_TASK_RETURN_OBJECT:
+          g_clear_object ((GObject **) &data->result);
+          break;
+
+        case FP_DEVICE_TASK_RETURN_PTR_ARRAY:
+          g_clear_pointer ((GPtrArray **) &data->result, g_ptr_array_unref);
+          break;
+
+        case FP_DEVICE_TASK_RETURN_ERROR:
+          g_clear_error ((GError **) &data->result);
+          break;
+
+        default:
+          g_assert_not_reached ();
+        }
+    }
   g_object_unref (data->device);
   g_free (data);
 }

libfprint/fpi-device.h

@@ -202,6 +202,7 @@ void fpi_device_get_delete_data (FpDevice *device,
                                  FpPrint **print);
 GCancellable *fpi_device_get_cancellable (FpDevice *device);
 
+void fpi_device_remove (FpDevice *device);
 
 GSource * fpi_device_add_timeout (FpDevice      *device,
                                   gint           interval,