From fe498c56c7aa39b3f3460664a2e605ade269de9a Mon Sep 17 00:00:00 2001 From: Benjamin Berg Date: Tue, 10 Nov 2020 15:56:30 +0100 Subject: [PATCH] virtual-image: Fix race condition closing new connection When a new connection came in we would close the old connection. This in turn would trigger a receive error causing the *new* connection to be closed from the error handler. Fix this by simply cancelling any pending transfers when a new connection comes in. Also change the error handling code to catch issues like partial writes correctly. This fixes an issue for the fprintd test where some tests were flaky. --- libfprint/drivers/virtual-image.c | 36 ++++++++++++++++++++----------- 1 file changed, 23 insertions(+), 13 deletions(-) diff --git a/libfprint/drivers/virtual-image.c b/libfprint/drivers/virtual-image.c index aa06080c..b5164f3f 100644 --- a/libfprint/drivers/virtual-image.c +++ b/libfprint/drivers/virtual-image.c @@ -73,14 +73,21 @@ recv_image_img_recv_cb (GObject *source_object, success = g_input_stream_read_all_finish (G_INPUT_STREAM (source_object), res, &bytes, &error); - if (!success || bytes == 0) + /* Can't use self if the operation was cancelled. */ + if (!success && g_error_matches (error, G_IO_ERROR, G_IO_ERROR_CANCELLED)) + return; + + self = FPI_DEVICE_VIRTUAL_IMAGE (user_data); + device = FP_IMAGE_DEVICE (self); + + /* Consider success if we received the right amount of data, otherwise + * an error must have happened. */ + if (bytes < self->recv_img->width * self->recv_img->height) { if (!success) - { - if (g_error_matches (error, G_IO_ERROR, G_IO_ERROR_CANCELLED)) - return; - g_warning ("Error receiving header for image data: %s", error->message); - } + g_warning ("Error receiving image data: %s", error->message); + else + g_warning ("Error receiving image data: end of stream before all data was read"); self = FPI_DEVICE_VIRTUAL_IMAGE (user_data); g_io_stream_close (G_IO_STREAM (self->connection), NULL, NULL); @@ -88,9 +95,6 @@ recv_image_img_recv_cb (GObject *source_object, return; } - self = FPI_DEVICE_VIRTUAL_IMAGE (user_data); - device = FP_IMAGE_DEVICE (self); - if (self->automatic_finger) fpi_image_device_report_finger_status (device, TRUE); fpi_image_device_image_captured (device, g_steal_pointer (&self->recv_img)); @@ -113,7 +117,7 @@ recv_image_hdr_recv_cb (GObject *source_object, success = g_input_stream_read_all_finish (G_INPUT_STREAM (source_object), res, &bytes, &error); - if (!success || bytes == 0) + if (!success || bytes != sizeof (self->recv_img_hdr)) { if (!success) { @@ -122,6 +126,10 @@ recv_image_hdr_recv_cb (GObject *source_object, return; g_warning ("Error receiving header for image data: %s", error->message); } + else if (bytes != 0) + { + g_warning ("Received incomplete header before end of stream."); + } self = FPI_DEVICE_VIRTUAL_IMAGE (user_data); g_io_stream_close (G_IO_STREAM (self->connection), NULL, NULL); @@ -235,10 +243,12 @@ new_connection_cb (GObject *source_object, GAsyncResult *res, gpointer user_data if (dev->connection) { /* We may not have noticed that the stream was closed, - * if the device is deactivated. Double check here. */ - g_input_stream_is_closed (g_io_stream_get_input_stream (G_IO_STREAM (dev->connection))); + * if the device is deactivated. + * Cancel any ongoing operation on the old connection. */ + g_cancellable_cancel (dev->cancellable); + g_clear_object (&dev->cancellable); + dev->cancellable = g_cancellable_new (); - g_io_stream_close (G_IO_STREAM (dev->connection), NULL, NULL); g_clear_object (&dev->connection); }