From 72a2504fc431aeacfc1a4883ac0de2b0a2b3b5c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Sat, 23 Jan 2021 22:22:42 +0100 Subject: [PATCH] device: Wait device to finish for a timeout before completing VerifyStop When a device has reported the verification status the client should call VerifyStop to stop the device, however this under the hood may lead to a premature cancellation, causing the device not to react as expected in case the finger is still on the sensor or in case it may return to us some errors that we may want to handle (like the data-missing one). So, in case we are about to stop the verification and the operation is still in process, wait for a maximum timeout before proceed to the cancellation. However, while waiting, the action may be also cancelled because of a call to Release() or because the client vanished, and in such case we have to ensure that the current invocation is saved for being invoked by stoppable_action_completed() when callback will return. That will also unset it, and that's a clear indication for us that it has been already consumed, and thus that we can just return doing nothing else. Fixes: #100 --- src/device.c | 71 ++++++++++++++++++++++++++++++++++++++++++++++++ tests/fprintd.py | 65 ++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 133 insertions(+), 3 deletions(-) diff --git a/src/device.c b/src/device.c index 76753b5..86e9b96 100644 --- a/src/device.c +++ b/src/device.c @@ -33,6 +33,8 @@ #include "fprintd.h" #include "storage.h" +#define VERIFY_STOP_DEVICE_WAIT 1 /* Seconds to wait for the device to complete */ + static const char *FINGERS_NAMES[] = { [FP_FINGER_UNKNOWN] = "unknown", [FP_FINGER_LEFT_THUMB] = "left-thumb", @@ -94,6 +96,8 @@ typedef struct FpDevice *dev; SessionData *_session; + guint verify_stop_wait_timeout_id; + PolkitAuthority *auth; /* Hashtable of connected clients */ @@ -235,6 +239,7 @@ fprint_device_finalize (GObject *object) FprintDevice *self = (FprintDevice *) object; FprintDevicePrivate *priv = fprint_device_get_instance_private (self); + g_clear_handle_id (&priv->verify_stop_wait_timeout_id, g_source_remove); g_hash_table_destroy (priv->clients); session_data_set_new (priv, NULL, NULL); g_clear_object (&priv->auth); @@ -1486,11 +1491,43 @@ fprint_device_verify_start (FprintDBusDevice *dbus_dev, return TRUE; } +static gboolean +verify_stop_wait_timeout (gpointer data) +{ + guint *timeout_id = data; + + *timeout_id = 0; + return FALSE; +} + +static gboolean +verify_has_completed (FprintDevice *rdev) +{ + FprintDevicePrivate *priv = fprint_device_get_instance_private (rdev); + + if (!priv->current_cancellable || + g_cancellable_is_cancelled (priv->current_cancellable)) + return TRUE; + + switch (priv->current_action) + { + case ACTION_VERIFY: + return !priv->verify_data; + + case ACTION_IDENTIFY: + return !priv->identify_data; + + default: + g_assert_not_reached (); + } +} + static gboolean fprint_device_verify_stop (FprintDBusDevice *dbus_dev, GDBusMethodInvocation *invocation) { FprintDevice *rdev = FPRINT_DEVICE (dbus_dev); + FprintDevicePrivate *priv = fprint_device_get_instance_private (rdev); g_autoptr(GError) error = NULL; @@ -1506,6 +1543,40 @@ fprint_device_verify_stop (FprintDBusDevice *dbus_dev, return TRUE; } + if (!verify_has_completed (rdev)) + { + g_autoptr(SessionData) session = session_data_get (priv); + + if (session->verify_status_reported) + { + /* If we got a status report we need to delay the cancellation + * of the action, leaving the device some more time to complete + * the operation (and in case return the real error) before proceed + * in cancelling it. + * In case Release or client vanished while waiting the invocation + * will be handled by stoppable_action_completed() during cancellation + */ + g_assert (priv->verify_stop_wait_timeout_id == 0); + + priv->verify_stop_wait_timeout_id = + g_timeout_add_seconds (VERIFY_STOP_DEVICE_WAIT, verify_stop_wait_timeout, + &priv->verify_stop_wait_timeout_id); + + g_assert (priv->current_cancel_invocation == NULL); + priv->current_cancel_invocation = invocation; + + while (priv->verify_stop_wait_timeout_id && !verify_has_completed (rdev)) + g_main_context_iteration (NULL, TRUE); + + g_clear_handle_id (&priv->verify_stop_wait_timeout_id, g_source_remove); + + if (!priv->current_cancel_invocation) + return TRUE; + + priv->current_cancel_invocation = NULL; + } + } + stoppable_action_stop (rdev, invocation); return TRUE; diff --git a/tests/fprintd.py b/tests/fprintd.py index d40ba77..eee3033 100644 --- a/tests/fprintd.py +++ b/tests/fprintd.py @@ -387,14 +387,17 @@ class FPrintdTest(dbusmock.DBusTestCase): with Connection(self.sockaddr) as con: self.send_image(image, con) - def send_finger_automatic(self, automatic, con=None): + def send_finger_automatic(self, automatic, con=None, iterate=True): # Set whether finger on/off is reported around images if con: con.sendall(struct.pack('ii', -3, 1 if automatic else 0)) return with Connection(self.sockaddr) as con: - self.send_finger_automatic(automatic, con=con) + self.send_finger_automatic(automatic, con=con, iterate=iterate) + + while iterate and ctx.pending(): + ctx.iteration(False) def send_finger_report(self, has_finger, con=None, iterate=True): # Send finger on/off @@ -1944,6 +1947,7 @@ class FPrintdVirtualDeviceVerificationTests(FPrintdVirtualDeviceBaseTest): cls.enroll_finger = 'left-middle-finger' cls.verify_finger = cls.enroll_finger cls.stop_on_teardown = True + cls.releases_on_teardown = True def setUp(self): super().setUp() @@ -1954,7 +1958,8 @@ class FPrintdVirtualDeviceVerificationTests(FPrintdVirtualDeviceBaseTest): def tearDown(self): if self.stop_on_teardown: self.device.VerifyStop() - self.device.Release() + if self.releases_on_teardown: + self.device.Release() super().tearDown() def assertVerifyRetry(self, device_error, expected_error): @@ -2097,6 +2102,60 @@ class FPrintdVirtualDeviceVerificationTests(FPrintdVirtualDeviceBaseTest): self.send_error(con=con) self.wait_for_result(max_wait=200, expected='verify-match') + def test_verify_stop_restarts_immediately(self): + self.send_image('tented_arch') + self.wait_for_result() + self.assertTrue(self._verify_stopped) + self.assertEqual(self._last_result, 'verify-no-match') + + self.call_device_method_async('VerifyStop', '()', []) + self.call_device_method_async('VerifyStart', '(s)', [self.verify_finger]) + + self.wait_for_device_reply(expected_replies=2) + + def test_verify_stop_waits_for_completion(self): + self.stop_on_teardown = False + + with Connection(self.sockaddr) as con: + self.send_finger_automatic(False, con=con) + self.send_finger_report(True, con=con) + self.send_image('tented_arch', con=con) + self.wait_for_result() + + self.assertTrue(self._verify_stopped) + self.assertEqual(self._last_result, 'verify-no-match') + + self.call_device_method_async('VerifyStop', '()', []) + + def restart_verify(abort=False): + self.call_device_method_async('VerifyStart', '(s)', [self.verify_finger]) + with self.assertFprintError('AlreadyInUse'): + self.wait_for_device_reply(method='VerifyStart') + + self.assertFalse(self.get_async_replies(method='VerifyStop')) + self._abort = abort + + restart_verify() + GLib.timeout_add(100, restart_verify) + GLib.timeout_add(300, restart_verify, True) + self.wait_for_result() + + def test_verify_stop_waits_for_completion_waiting_timeout(self): + self.test_verify_stop_waits_for_completion() + self.wait_for_device_reply(method='VerifyStop') + self.assertTrue(self.get_async_replies(method='VerifyStop')) + + def test_verify_stop_waits_for_completion_is_stopped_by_release(self): + # During the release here we're testing the case in which + # while we're waiting for VerifyStop to return, Release stops the + # verification, making the invocation to return + self.releases_on_teardown = False + self.test_verify_stop_waits_for_completion() + self.assertFalse(self.get_async_replies(method='VerifyStop')) + self.call_device_method_async('Release', '()', []) + self.wait_for_device_reply(method='Release') + self.assertTrue(self.get_async_replies(method='VerifyStop')) + class FPrintdVirtualDeviceIdentificationTests(FPrintdVirtualDeviceVerificationTests): '''This class will just repeat the tests of FPrintdVirtualDeviceVerificationTests