diff --git a/src/libei-device.c b/src/libei-device.c index be083f5..9a74903 100644 --- a/src/libei-device.c +++ b/src/libei-device.c @@ -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); diff --git a/src/libei-private.h b/src/libei-private.h index bbf4677..5b6778f 100644 --- a/src/libei-private.h +++ b/src/libei-private.h @@ -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); diff --git a/src/libei-seat.c b/src/libei-seat.c index e75a8ec..ec695b3 100644 --- a/src/libei-seat.c +++ b/src/libei-seat.c @@ -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); -} diff --git a/src/libei.c b/src/libei.c index a400cb2..e4a72e0 100644 --- a/src/libei.c +++ b/src/libei.c @@ -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; } diff --git a/src/libeis-private.h b/src/libeis-private.h index fbc5701..75a93af 100644 --- a/src/libeis-private.h +++ b/src/libeis-private.h @@ -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;