Force the device locker to ->close() an aborted ->open()

Fixes https://github.com/fwupd/fwupd/issues/3187

libfwupdplugin/fu-device-locker.c

@@ -202,8 +202,14 @@ fu_device_locker_new_full (gpointer device,
 	self->close_func = close_func;
 
 	/* open device */
-	if (!self->open_func (device, error))
+	if (!self->open_func (device, error)) {
+		g_autoptr(GError) error_local = NULL;
+		if (!self->close_func (device, &error_local)) {
+			g_debug ("ignoring close error on aborted open: %s",
+				 error_local->message);
+		}
 		return NULL;
+	}
 
 	/* success */
 	self->device_open = TRUE;

libfwupdplugin/fu-device.c

@@ -3337,6 +3337,13 @@ fu_device_open_cb (FuDevice *self, gpointer user_data, GError **error)
  * It is expected that plugins issue the same number of fu_device_open() and
  * fu_device_close() methods when using a specific @self.
  *
+ * If the `->probe()`, `->open()` and `->setup()` actions all complete
+ * successfully the internal device flag %FU_DEVICE_INTERNAL_FLAG_IS_OPEN will
+ * be set.
+ *
+ * NOTE: It is important to still call fu_device_close() even if this function
+ * fails as the device may still be partially initialized.
+ *
  * Returns: %TRUE for success
  *
  * Since: 1.1.2
@@ -3386,6 +3393,7 @@ fu_device_open (FuDevice *self, GError **error)
 		return FALSE;
 
 	/* success */
+	fu_device_add_internal_flag (self, FU_DEVICE_INTERNAL_FLAG_IS_OPEN);
 	return TRUE;
 }
 
@@ -3405,6 +3413,9 @@ fu_device_open (FuDevice *self, GError **error)
  * An error is returned if this method is called without having used the
  * fu_device_open() method beforehand.
  *
+ * If the close action completed successfully the internal device flag
+ * %FU_DEVICE_INTERNAL_FLAG_IS_OPEN will be cleared.
+ *
  * Returns: %TRUE for success
  *
  * Since: 1.1.2
@@ -3436,6 +3447,7 @@ fu_device_close (FuDevice *self, GError **error)
 	}
 
 	/* success */
+	fu_device_remove_internal_flag (self, FU_DEVICE_INTERNAL_FLAG_IS_OPEN);
 	return TRUE;
 }
 

libfwupdplugin/fu-device.h

@@ -224,6 +224,7 @@ FuDevice	*fu_device_new				(void);
  * @FU_DEVICE_INTERNAL_FLAG_RETRY_OPEN:			Retry the device open up to 5 times if it fails
  * @FU_DEVICE_INTERNAL_FLAG_REPLUG_MATCH_GUID:		Match GUIDs on device replug where the physical and logical IDs will be different
  * @FU_DEVICE_INTERNAL_FLAG_INHERIT_ACTIVATION:		Inherit activation status from the history database on startup
+ * @FU_DEVICE_INTERNAL_FLAG_IS_OPEN:			The device opened successfully and ready to use
  *
  * The device internal flags.
  **/
@@ -239,6 +240,7 @@ typedef enum {
 	FU_DEVICE_INTERNAL_FLAG_RETRY_OPEN		= (1llu << 7),	/* Since: 1.5.5 */
 	FU_DEVICE_INTERNAL_FLAG_REPLUG_MATCH_GUID	= (1llu << 8),	/* Since: 1.5.8 */
 	FU_DEVICE_INTERNAL_FLAG_INHERIT_ACTIVATION	= (1llu << 9),  /* Since: 1.5.9 */
+	FU_DEVICE_INTERNAL_FLAG_IS_OPEN			= (1llu << 10),	/* Since: 1.6.1 */
 	/*< private >*/
 	FU_DEVICE_INTERNAL_FLAG_UNKNOWN			= G_MAXUINT64,
 } FuDeviceInternalFlags;

libfwupdplugin/fu-self-test.c

@@ -695,17 +695,19 @@ fu_device_locker_func (void)
 }
 
 static gboolean
-_fail_open_cb (GObject *device, GError **error)
+_fail_open_cb (FuDevice *device, GError **error)
 {
+	fu_device_set_metadata_boolean (device, "Test::Open", TRUE);
 	g_set_error_literal (error, G_IO_ERROR, G_IO_ERROR_FAILED, "fail");
 	return FALSE;
 }
 
 static gboolean
-_fail_close_cb (GObject *device, GError **error)
+_fail_close_cb (FuDevice *device, GError **error)
 {
-	g_assert_not_reached ();
+	fu_device_set_metadata_boolean (device, "Test::Close", TRUE);
-	return TRUE;
+	g_set_error_literal (error, G_IO_ERROR, G_IO_ERROR_BUSY, "busy");
+	return FALSE;
 }
 
 static void
@@ -713,10 +715,16 @@ fu_device_locker_fail_func (void)
 {
 	g_autoptr(FuDeviceLocker) locker = NULL;
 	g_autoptr(GError) error = NULL;
-	g_autoptr(GObject) device = g_object_new (G_TYPE_OBJECT, NULL);
+	g_autoptr(FuDevice) device = fu_device_new ();
-	locker = fu_device_locker_new_full (device, _fail_open_cb, _fail_close_cb, &error);
+	locker = fu_device_locker_new_full (device,
+					    (FuDeviceLockerFunc) _fail_open_cb,
+					    (FuDeviceLockerFunc) _fail_close_cb,
+					    &error);
 	g_assert_error (error, G_IO_ERROR, G_IO_ERROR_FAILED);
 	g_assert_null (locker);
+	g_assert_true (fu_device_get_metadata_boolean (device, "Test::Open"));
+	g_assert_true (fu_device_get_metadata_boolean (device, "Test::Close"));
+	g_assert_false (fu_device_has_internal_flag (device, FU_DEVICE_INTERNAL_FLAG_IS_OPEN));
 }
 
 static void