fdo-mirrors/libei
1
0
Fork 0
mirror of https://gitlab.freedesktop.org/libinput/libei.git synced 2026-01-10 12:40:14 +01:00
Code Issues Projects Releases Packages Wiki Activity Actions

libei: fix the device/seat ref handling once again

Browse source 
Previously we didn't always clean up properly, especially where unexpected
removals happened. So the new approach is:
- the device always has a ref to the seat, we must not remove the seat until
  even not-yet-added devices are released
- the seat has a ref to the device *after* it was added. this is a circular
  ref so we need to make sure the device is manually removed from the seat
  so we can actually reach a refcount 1

This is made slightly more complicated by us calling ei_disconnect() whenever
we fail to write on the fd. The result is re-entrant functions that we need to
protect against inadvertent changes. The best option here is probably to mark
the context as degraded and clean up once we finished whatever we were really
doing - that's a larger rework though.

Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
This commit is contained in:
Peter Hutterer 2020-10-28 15:32:42 +10:00
parent 96109fb415
commit eef9c51df3
5 changed files with 106 additions and 75 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

87
src/libei-device.c
View file

@ -59,23 +59,40 @@ ei_device_set_state(struct ei_device *device,
ei_device_state_to_string(state));
}
static void
ei_device_add_to_seat(struct ei_device *device)
{
struct ei_seat *seat = ei_device_get_seat(device);
list_remove(&device->link); /* remove from pending */
list_append(&seat->devices, &device->link);
/* Once the seat has the device proper, it owns a ref to the
* device (and the device always owns a ref to the seat)
*/
ei_device_ref(device);
ei_device_set_state(device, EI_DEVICE_STATE_CONNECTING);
}
static void
ei_device_remove_from_seat(struct ei_device *device)
{
list_remove(&device->link);
list_init(&device->link);
/* seat only has a ref for devices already added */
if (device->state != EI_DEVICE_STATE_NEW)
ei_device_unref(device);
}
static void
ei_device_destroy(struct ei_device *device)
{
struct ei_seat *seat = ei_device_get_seat(device);
struct ei_device *d;
/* If the device was still pending, the client held the only ref to
* it. Any other case, the device should've been removed from the
* seat by the time we get here. */
list_for_each(d, &seat->devices_pending, link) {
if (device == d) {
list_remove(&d->link);
break;
}
}
if (device->state == EI_DEVICE_STATE_NEW)
ei_device_remove_from_seat(device);
ei_keymap_unref(device->keymap);
ei_seat_unref(seat);
free(device->name);
}
@ -123,6 +140,11 @@ ei_device_new(struct ei_seat *seat)
list_append(&seat->devices_pending, &device->link); /* not a ref! */
/* We have a ref to the seat to make sure our seat doesn't get
* destroyed while a ref to the device is still alive.
*/
ei_seat_ref(seat);
return device;
}
@ -310,16 +332,29 @@ ei_device_set_keymap(struct ei_device *device,
_public_ void
ei_device_add(struct ei_device *device)
{
switch (device->state) {
case EI_DEVICE_STATE_NEW:
break;
case EI_DEVICE_STATE_REMOVED_FROM_CLIENT:
case EI_DEVICE_STATE_REMOVED_FROM_SERVER:
case EI_DEVICE_STATE_CONNECTING:
case EI_DEVICE_STATE_SUSPENDED:
case EI_DEVICE_STATE_RESUMED:
case EI_DEVICE_STATE_DEAD:
log_bug_client(ei_device_get_context(device),
"%s: device %s added twice by client\n",
__func__, device->name);
return;
}
if (ei_device_has_capability(device, EI_DEVICE_CAP_POINTER_ABSOLUTE) &&
(device->abs.dim.width == 0 || device->abs.dim.height == 0))
log_bug_client(ei_device_get_context(device),
"%s: device %s is missing an abs pointer range\n",
__func__, device->name);
if (ei_send_add_device(device) == 0) {
ei_device_set_state(device, EI_DEVICE_STATE_CONNECTING);
ei_seat_add_device(device);
}
if (ei_send_add_device(device) == 0)
ei_device_add_to_seat(device);
}
_public_ void
@ -328,22 +363,19 @@ ei_device_remove(struct ei_device *device)
switch (device->state) {
case EI_DEVICE_STATE_NEW:
case EI_DEVICE_STATE_DEAD:
break;
case EI_DEVICE_STATE_REMOVED_FROM_CLIENT:
log_bug_client(ei_device_get_context(device),
"%s: device %s removed twice by client\n",
__func__, device->name);
break;;
break;
case EI_DEVICE_STATE_REMOVED_FROM_SERVER:
/* device is properly dead now */
ei_send_remove_device(device);
ei_device_set_state(device, EI_DEVICE_STATE_DEAD);
ei_device_remove_from_seat(device);
ei_send_remove_device(device);
break;
case EI_DEVICE_STATE_CONNECTING:
case EI_DEVICE_STATE_SUSPENDED:
case EI_DEVICE_STATE_RESUMED:
ei_send_remove_device(device);
ei_device_set_state(device, EI_DEVICE_STATE_REMOVED_FROM_CLIENT);
ei_send_remove_device(device);
break;
}
}
@ -355,23 +387,19 @@ ei_device_removed_by_server(struct ei_device *device)
case EI_DEVICE_STATE_NEW:
case EI_DEVICE_STATE_DEAD:
case EI_DEVICE_STATE_REMOVED_FROM_SERVER:
log_bug(ei_device_get_context(device),
"%s: device %s removed in state %s\n",
__func__, device->name,
ei_device_state_to_string(device->state));
break;
case EI_DEVICE_STATE_REMOVED_FROM_CLIENT:
/* device is properly dead now */
ei_queue_device_removed_event(device);
ei_device_set_state(device, EI_DEVICE_STATE_DEAD);
ei_seat_remove_device(device);
ei_device_remove_from_seat(device);
break;
case EI_DEVICE_STATE_CONNECTING:
/* Device never seen by server, but our client still thinks
* it's there */
ei_queue_device_removed_event(device);
ei_device_set_state(device, EI_DEVICE_STATE_DEAD);
ei_seat_remove_device(device);
ei_device_remove_from_seat(device);
break;
case EI_DEVICE_STATE_SUSPENDED:
case EI_DEVICE_STATE_RESUMED:
@ -620,15 +648,16 @@ ei_touch_up(struct ei_touch *touch)
#define FAKE_SEAT(_seat) \
struct ei_seat seat_##__LINE__ = { \
.object = { .refcount = 1, }, \
.id = __LINE__ << 16, \
.capabilities = ~0, \
.name = "default", \
}; \
list_init(&seat_##__LINE__.link); \
list_init(&seat_##__LINE__.devices); \
list_init(&seat_##__LINE__.devices_pending); \
struct ei_seat *_seat = &seat_ ## __LINE__;
MUNIT_TEST(test_device_new)
{
FAKE_SEAT(seat);

14
src/libei-private.h
View file

@ -64,10 +64,16 @@ struct ei {
} log;
};
enum ei_seat_state {
EI_SEAT_STATE_PRESENT,
EI_SEAT_STATE_REMOVED,
};
struct ei_seat {
struct object object;
void *user_data;
struct list link;
enum ei_seat_state state;
/* devices created by client but not yet added */
struct list devices_pending;
struct list devices;
@ -166,10 +172,7 @@ void
ei_seat_remove(struct ei_seat *seat);
void
ei_seat_add_device(struct ei_device *device);
void
ei_seat_remove_device(struct ei_device *device);
ei_seat_drop(struct ei_seat *seat);
int
ei_send_add_device(struct ei_device *device);
@ -180,6 +183,9 @@ ei_send_remove_device(struct ei_device *device);
void
ei_queue_device_removed_event(struct ei_device *device);
void
ei_queue_seat_removed_event(struct ei_seat *seat);
void
ei_device_removed_by_server(struct ei_device *device);

42
src/libei-seat.c
View file

@ -65,6 +65,7 @@ ei_seat_new(struct ei *ei, uint32_t id, const char *name, uint32_t capabilities)
{
struct ei_seat *seat = ei_seat_create(&ei->object);
seat->state = EI_SEAT_STATE_PRESENT;
seat->name = xstrdup(name);
seat->id = id;
seat->capabilities = capabilities;
@ -79,28 +80,34 @@ ei_seat_new(struct ei *ei, uint32_t id, const char *name, uint32_t capabilities)
void
ei_seat_remove(struct ei_seat *seat)
{
/* Sigh, this is terrible and needs to be fixed:
* if our fd is broken, trying to send any event causes an ei_disconnect(),
* which eventually calls in here. So we need to guard this function
* against nested callers. */
if (seat->state == EI_SEAT_STATE_REMOVED)
return;
struct ei_device *d, *tmp;
/* If the server disconnects us before processing a new device, we
* need to clean this up in the library */
list_for_each_safe(d, tmp, &seat->devices, link) {
/* remove the device */
ei_device_remove(d);
/* And pretend to process the removed message from
* the server */
ei_device_removed_by_server(d);
}
list_remove(&seat->link);
list_init(&seat->link);
}
void
ei_seat_add_device(struct ei_device *device)
{
struct ei_seat *seat = ei_device_get_seat(device);
/* the seat owns the ref to the device */
ei_device_ref(device);
/* remove from pending list */
list_remove(&device->link);
list_append(&seat->devices, &device->link);
/* Check the seat state again, because the above device removal may
* have triggered ei_disconnect() */
if (seat->state != EI_SEAT_STATE_REMOVED) {
seat->state = EI_SEAT_STATE_REMOVED;
list_remove(&seat->link);
list_init(&seat->link);
ei_queue_seat_removed_event(seat);
ei_seat_unref(seat);
}
}
struct ei_device *
@ -119,10 +126,3 @@ ei_seat_find_device(struct ei_seat *seat, uint32_t deviceid)
}
return NULL;
}
void
ei_seat_remove_device(struct ei_device *device)
{
list_remove(&device->link);
ei_device_unref(device);
}

36
src/libei.c
View file

@ -231,6 +231,12 @@ queue_seat_removed_event(struct ei_seat *seat)
queue_event(ei, e);
}
void
ei_queue_seat_removed_event(struct ei_seat *seat)
{
queue_seat_removed_event(seat);
}
static void
queue_device_added_event(struct ei_device *device)
{
@ -395,14 +401,6 @@ connection_send_touch_up(struct ei *ei, struct ei_device *device, uint32_t tid)
return ei_proto_send_touch_up(ei, device, tid);
}
static void
ei_drop_seat(struct ei_seat *seat)
{
queue_seat_removed_event(seat);
list_remove(&seat->link);
ei_seat_unref(seat);
}
void
ei_disconnect(struct ei *ei)
{
@ -418,16 +416,7 @@ ei_disconnect(struct ei *ei)
struct ei_seat *seat, *tmps;
list_for_each_safe(seat, tmps, &ei->seats, link) {
struct ei_device *d, *tmpd;
list_for_each_safe(d, tmpd, &seat->devices, link) {
/* remove the device */
ei_device_remove(d);
/* And pretend to process the removed message from
* the server */
ei_device_removed_by_server(d);
}
ei_seat_remove(seat);
ei_drop_seat(seat);
}
if (state != EI_STATE_NEW) {
@ -481,13 +470,11 @@ handle_msg_seat_added(struct ei *ei, uint32_t seatid,
static int
handle_msg_seat_removed(struct ei *ei, uint32_t seatid)
{
log_debug(ei, "Removed seat %#x\n", seatid);
log_debug(ei, "server removed seat %#x\n", seatid);
struct ei_seat *seat = ei_find_seat(ei, seatid);
if (seat) {
ei_seat_remove(seat);
queue_seat_removed_event(seat);
ei_seat_unref(seat);
}
return 0;
@ -798,7 +785,16 @@ connection_message_callback(struct brei_message *bmsg, void *userdata)
break;
case EI_STATE_DISCONNECTING:
case EI_STATE_DISCONNECTED:
#if 0
/* FIXME: this shouldn't happen, but right now we
* may end up calling ei_disconnect() on an error
* which cleans most things up, leaving us with the
* message processing once we have actually cleaned
* up. Needs a bigger rework than currently
* possible.
*/
assert(!"Protocol error: message received while disconnecting\n");
#endif
break;
}

2
src/libeis-private.h
View file

@ -112,7 +112,7 @@ enum eis_device_state {
struct eis_device {
struct object object; /* parent is ei_seat */
struct object object; /* parent is ei_seat, and we have a ref to it */
struct list link;
uint32_t id;
char *name;