From 0051ff63521925dacd4a91b22a99201f3745395d Mon Sep 17 00:00:00 2001
From: Benjamin Berg <bberg@redhat.com>
Date: Mon, 16 Nov 2020 19:16:53 +0100
Subject: [PATCH] device: Treat devices as closed even after a close failure

We require the close call, but as the underlying transport layer is
gone, it will generally just return an error.

In principle, it makes sense to think of close as a function that always
succeeds (i.e. it makes no sense to try again). Should the device be in
a bad state, then a subsequent open() will simply fail.
---
 libfprint/fpi-device.c  | 18 +++++++++---------
 tests/test-fpi-device.c | 36 +++++++++++++++++++-----------------
 2 files changed, 28 insertions(+), 26 deletions(-)

diff --git a/libfprint/fpi-device.c b/libfprint/fpi-device.c
index 3aa65f3..981df87 100644
--- a/libfprint/fpi-device.c
+++ b/libfprint/fpi-device.c
@@ -976,17 +976,17 @@ fpi_device_close_complete (FpDevice *device, GError *error)
       return;
     }
 
+  /* Always consider the device closed. Drivers should try hard to close the
+   * device. Generally, e.g. cancellations should be ignored.
+   */
+  priv->is_open = FALSE;
+  g_object_notify (G_OBJECT (device), "open");
+
   if (!error)
-    {
-      priv->is_open = FALSE;
-      g_object_notify (G_OBJECT (device), "open");
-      fpi_device_return_task_in_idle (device, FP_DEVICE_TASK_RETURN_BOOL,
-                                      GUINT_TO_POINTER (TRUE));
-    }
+    fpi_device_return_task_in_idle (device, FP_DEVICE_TASK_RETURN_BOOL,
+                                    GUINT_TO_POINTER (TRUE));
   else
-    {
-      fpi_device_return_task_in_idle (device, FP_DEVICE_TASK_RETURN_ERROR, error);
-    }
+    fpi_device_return_task_in_idle (device, FP_DEVICE_TASK_RETURN_ERROR, error);
 }
 
 /**
diff --git a/tests/test-fpi-device.c b/tests/test-fpi-device.c
index 2b6b256..43e84f7 100644
--- a/tests/test-fpi-device.c
+++ b/tests/test-fpi-device.c
@@ -588,7 +588,7 @@ test_driver_close_error (void)
   g_assert (fake_dev->last_called_function == dev_class->close);
   g_assert_error (error, FP_DEVICE_ERROR, FP_DEVICE_ERROR_GENERAL);
   g_assert (error == g_steal_pointer (&fake_dev->ret_error));
-  g_assert_true (fp_device_is_open (device));
+  g_assert_false (fp_device_is_open (device));
 }
 
 static void
@@ -2069,12 +2069,6 @@ test_driver_action_error_all (void)
 
   fake_dev->return_action_error = TRUE;
 
-  fake_dev->ret_error = fpi_device_error_new (FP_DEVICE_ERROR_DATA_INVALID);
-  g_assert_false (fp_device_close_sync (device, NULL, &error));
-  g_assert_true (fake_dev->last_called_function == dev_class->close);
-  g_assert_error (error, FP_DEVICE_ERROR, FP_DEVICE_ERROR_DATA_INVALID);
-  g_clear_error (&error);
-
   fake_dev->ret_error = fpi_device_error_new (FP_DEVICE_ERROR_DATA_INVALID);
   g_assert_null (fp_device_enroll_sync (device, fp_print_new (device), NULL,
                                         NULL, NULL, &error));
@@ -2113,6 +2107,13 @@ test_driver_action_error_all (void)
   g_assert_true (fake_dev->last_called_function == dev_class->delete);
   g_assert_error (error, FP_DEVICE_ERROR, FP_DEVICE_ERROR_DATA_INVALID);
   g_clear_error (&error);
+
+  /* Test close last, as we can't operate on a closed device. */
+  fake_dev->ret_error = fpi_device_error_new (FP_DEVICE_ERROR_DATA_INVALID);
+  g_assert_false (fp_device_close_sync (device, NULL, &error));
+  g_assert_true (fake_dev->last_called_function == dev_class->close);
+  g_assert_error (error, FP_DEVICE_ERROR, FP_DEVICE_ERROR_DATA_INVALID);
+  g_clear_error (&error);
 }
 
 static void
@@ -2146,16 +2147,6 @@ test_driver_action_error_fallback_all (void)
                          "error function*");
 
   fake_dev->return_action_error = TRUE;
-  g_assert_false (fp_device_close_sync (device, NULL, &error));
-  g_test_assert_expected_messages ();
-  g_assert_true (fake_dev->last_called_function == dev_class->close);
-  g_assert_error (error, FP_DEVICE_ERROR, FP_DEVICE_ERROR_GENERAL);
-  g_clear_error (&error);
-
-  g_test_expect_message (G_LOG_DOMAIN, G_LOG_LEVEL_WARNING,
-                         "*Device failed to pass an error to generic action "
-                         "error function*");
-
   g_assert_null (fp_device_enroll_sync (device, fp_print_new (device), NULL,
                                         NULL, NULL, &error));
   g_test_assert_expected_messages ();
@@ -2215,6 +2206,17 @@ test_driver_action_error_fallback_all (void)
   g_assert_true (fake_dev->last_called_function == dev_class->delete);
   g_assert_error (error, FP_DEVICE_ERROR, FP_DEVICE_ERROR_GENERAL);
   g_clear_error (&error);
+
+  /* Test close last, as we can't operate on a closed device. */
+  g_test_expect_message (G_LOG_DOMAIN, G_LOG_LEVEL_WARNING,
+                         "*Device failed to pass an error to generic action "
+                         "error function*");
+
+  g_assert_false (fp_device_close_sync (device, NULL, &error));
+  g_test_assert_expected_messages ();
+  g_assert_true (fake_dev->last_called_function == dev_class->close);
+  g_assert_error (error, FP_DEVICE_ERROR, FP_DEVICE_ERROR_GENERAL);
+  g_clear_error (&error);
 }
 
 static void