Skip to content

Handle added/removed device events as Clutter Events

When a device is removed from the seat the events that this device may have emitted just before being removed might still be in the stage events queue, this may lead a to a crash because:

  • Once the device is removed, we dispose it and the staling event is kept in queue and sent for processing at next loop.
  • During event processing we ask the backend to update the last device with the disposed device
  • The device is disposed once the events referencing it, are free'd
  • The actual last device emission happens in an idle, but at this point the device may have been free'd, but in any case will be still disposed and so not providing useful informations.

To avoid this, once a device has been added/removed from the seat, we queue ClutterDeviceEvent events to inform the stack that the device state has changed, preserving the order with the other actual generated device events. In this way it can't happen that we emit another event before that the device has been added or after that it has been removed.


Now, this could lead to ignoring the latest events that the device sent, personally not sure this is too relevant, but if we do care about them another way would be to do something like the code below (to be used instead of commit c3eb33de ), in order to:

  • Process the queued events immediately (or maybe do it only if one is coming from the removed device?)
  • Notify the device as the last one without waiting (but, do we care?).
diff --git a/clutter/clutter/clutter-stage.c b/clutter/clutter/clutter-stage.c
index 454e6942e..7e3f4dc98 100644
--- a/clutter/clutter/clutter-stage.c
+++ b/clutter/clutter/clutter-stage.c
@@ -2029,6 +2029,7 @@ clutter_stage_init (ClutterStage *self)
   ClutterStagePrivate *priv;
   ClutterStageWindow *impl;
   ClutterBackend *backend;
+  ClutterSeat *seat;
   GError *error;
 
   /* a stage is a top-level object */
@@ -2066,6 +2067,12 @@ clutter_stage_init (ClutterStage *self)
   priv->sync_delay = -1;
   priv->motion_events_enabled = TRUE;
 
+  seat = clutter_backend_get_default_seat (backend);
+  g_signal_connect_object (seat, "device-removed",
+                           G_CALLBACK (_clutter_stage_process_queued_events),
+                           self,
+                           G_CONNECT_SWAPPED | G_CONNECT_AFTER);
+
   clutter_actor_set_background_color (CLUTTER_ACTOR (self),
                                       &default_stage_color);
 
diff --git a/src/backends/meta-backend.c b/src/backends/meta-backend.c
index 7694d90a7..cfcbcb019 100644
--- a/src/backends/meta-backend.c
+++ b/src/backends/meta-backend.c
@@ -1286,6 +1288,8 @@ meta_backend_update_last_device (MetaBackend        *backend,
                                  ClutterInputDevice *device)
 {
   MetaBackendPrivate *priv = meta_backend_get_instance_private (backend);
+  g_autoptr(GList) devices = NULL;
+  ClutterSeat *seat;
 
   if (priv->current_device == device)
     return;
@@ -1296,7 +1300,15 @@ meta_backend_update_last_device (MetaBackend        *backend,
 
   priv->current_device = device;
 
-  if (priv->device_update_idle_id == 0)
+  seat = clutter_backend_get_default_seat (priv->clutter_backend);
+  devices = clutter_seat_list_devices (seat);
+  if (!g_list_find (devices, device))
+    {
+      /* The device has just been removed, so let's update it promptly */
+      g_clear_handle_id (&priv->device_update_idle_id, g_source_remove);
+      update_last_device (backend); /* or just return earlier without setting the current device!? */
+    }
+  else if (priv->device_update_idle_id == 0)
     {
       priv->device_update_idle_id =
         g_idle_add ((GSourceFunc) update_last_device, backend);

Note that, not to crash, per sé we could just use g_set_object / g_clear_object to keep a ref on the priv->current_device, however this would not mute the errors we get at JS level, as the device would still be considered disposed at the moment the idle runs, and so we'd still complain at gjs level and emit using a null device object instead.

And in general, given we consider the device as "gone" once we get the removed signal, we should probably avoid to pass it around on signals once that has happened.

Edited by Robert Mader

Merge request reports