fdo-mirrors/libei
1
0
Fork 0
mirror of https://gitlab.freedesktop.org/libinput/libei.git synced 2026-04-05 11:30:41 +02:00
Code Issues Projects Releases Packages Wiki Activity Actions

Fix client-side removal of devices

Browse source 
Because events may be in-transit when a client removes the device, we need to
make this a full roundtrip to the server. Otherwise the client may assume a
device is removed, releases all references and then gets the original device
added event for that device. Better to have this as a round-trip instead.

This requires the server to call eis_device_disconnect() on the removed
notifications but we do so during eis_event_unref() anyway in case the server
forgets.

And it changes some of the API behaviors, so adjust the tests for that.

Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
This commit is contained in:
Peter Hutterer 2020-09-23 11:39:58 +10:00
parent 08da49debf
commit eb40872770
8 changed files with 184 additions and 29 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

7
src/libei-device.c
View file

@ -218,8 +218,13 @@ ei_device_add(struct ei_device *device)
_public_ void
ei_device_remove(struct ei_device *device)
{
if (device->state == EI_DEVICE_STATE_REMOVED)
switch (device->state) {
case EI_DEVICE_STATE_NEW:
case EI_DEVICE_STATE_REMOVED:
return;
default:
break;
}
ei_device_set_state(device, EI_DEVICE_STATE_REMOVED);
ei_remove_device(device);

35
src/libei.c
View file

@ -291,6 +291,25 @@ connection_send_key(struct ei *ei, struct ei_device *device,
return ei_proto_send_key(ei, device, k, is_press);
}
static void
ei_drop_device(struct ei_device *device)
{
switch (device->state) {
case EI_DEVICE_STATE_NEW:
break;
/* In any of these states the client has added the device, so we
* must send the REMOVED */
case EI_DEVICE_STATE_CONNECTING:
case EI_DEVICE_STATE_SUSPENDED:
case EI_DEVICE_STATE_RESUMED:
case EI_DEVICE_STATE_REMOVED:
queue_removed_event(device);
break;
}
list_remove(&device->link);
ei_device_unref(device);
}
void
ei_disconnect(struct ei *ei)
{
@ -306,7 +325,11 @@ ei_disconnect(struct ei *ei)
struct ei_device *d, *tmp;
list_for_each_safe(d, tmp, &ei->devices, link) {
/* remove the device */
ei_device_remove(d);
/* And pretend to process the removed message from the
* server */
ei_drop_device(d);
}
if (state != EI_STATE_NEW) {
@ -340,15 +363,9 @@ ei_remove_device(struct ei_device *device)
{
struct ei *ei = ei_device_get_context(device);
int rc = connection_send_remove(ei, device);
if (ei->state == EI_STATE_DISCONNECTING)
queue_removed_event(device);
list_remove(&device->link);
ei_device_unref(device);
if (rc)
ei_disconnect(ei);
return rc;
}
@ -387,9 +404,7 @@ handle_msg_removed(struct ei *ei, uint32_t deviceid)
list_for_each(d, &ei->devices, link) {
if (d->id == deviceid) {
queue_removed_event(d);
list_remove(&d->link);
ei_device_unref(d);
ei_drop_device(d);
break;
}
}

7
src/libei.h
View file

@ -456,9 +456,10 @@ ei_device_add(struct ei_device *device);
/**
* Notify the server that the device should be removed.
*
* The server will **not** send an @ref EI_EVENT_DEVICE_REMOVED event for
* this device, it should be considered removed by the client once this
* function completes.
* Due to the asynchronous nature of the client-server interaction,
* events for this device may still be in transit. The server will send an
* @ref EI_EVENT_DEVICE_REMOVED event for this device. After that event,
* device is considered removed by the server.
*
* This does not release any resources associated with this device, use
* ei_device_unref() for any references held by the client.

16
src/libeis-client.c
View file

@ -200,6 +200,20 @@ client_new_device(struct eis_client *client,
return 0;
}
static int
client_remove_device(struct eis_client *client, uint32_t deviceid)
{
struct eis_device *device;
list_for_each(device, &client->devices, link) {
if (device->id == deviceid) {
eis_queue_removed_event(device);
break;
}
}
return 0;
}
static int
client_pointer_rel(struct eis_client *client, uint32_t deviceid,
double x, double y)
@ -370,7 +384,7 @@ client_connected_handle_msg(struct eis_client *client,
msg->add_device.keymap_size);
break;
case MESSAGE_REMOVE_DEVICE:
/* FIXME: remove device */
rc = client_remove_device(client, msg->remove_device.deviceid);
break;
case MESSAGE_POINTER_REL:
rc = client_pointer_rel(client, msg->pointer_rel.deviceid,

4
src/libeis-event.c
View file

@ -37,11 +37,13 @@ eis_event_destroy(struct eis_event *event)
case EIS_EVENT_CLIENT_CONNECT:
case EIS_EVENT_CLIENT_DISCONNECT:
case EIS_EVENT_DEVICE_ADDED:
case EIS_EVENT_DEVICE_REMOVED:
case EIS_EVENT_POINTER_BUTTON:
case EIS_EVENT_POINTER_MOTION:
case EIS_EVENT_KEYBOARD_KEY:
break;
case EIS_EVENT_DEVICE_REMOVED:
eis_device_disconnect(event->device);
break;
default:
abort(); /* not yet implemented */
}

3
src/libeis.h
View file

@ -68,7 +68,8 @@ enum eis_event_type {
*/
EIS_EVENT_DEVICE_ADDED,
/**
* The device created by the client was removed.
* The device created by the client was removed. The server must
* call eis_device_disconnect() in response to this device.
*
* libeis guarantees this event is generated before
* @ref EIS_EVENT_CLIENT_DISCONNECT.

131
test/test-ei.c
View file

@ -107,8 +107,6 @@ MUNIT_TEST(test_ei_disconnect_after_add_before_received)
eis_client_disconnect(client);
}
/* But from the POV of the client we have added our devices, so we
* must receive a Removed event. */
with_client(peck) {
ei_dispatch(ei);
_cleanup_ei_event_ struct ei_event *removed =
@ -199,16 +197,17 @@ MUNIT_TEST(test_ei_disconnect_after_remove_before_received)
ei_device_remove(device);
}
/* Disconnect before the server processed the Removed message. This
* does **not** trigger a EI_EVENT_REMOVED, the device has already
* been removed by the client.
*/
/* No server dispatch here so the server isn't aware of the
* ei_device_remove() call. Disconnect the client, this
* automatically removes all devices */
with_server(peck) {
eis_client_disconnect(client);
}
with_client(peck) {
ei_dispatch(ei);
_cleanup_ei_event_ struct ei_event *removed =
peck_ei_next_event(ei, EI_EVENT_DEVICE_REMOVED);
_cleanup_ei_event_ struct ei_event *disconnect =
peck_ei_next_event(ei, EI_EVENT_DISCONNECT);
}
@ -253,10 +252,7 @@ MUNIT_TEST(test_ei_disconnect_after_remove_after_received)
ei_device_remove(device);
}
/* Disconnect after the server processed the Removed message. This
* does **not** trigger a EI_EVENT_REMOVED, the device has already
* been removed by the client.
*/
/* Dispatch, server is aware of the ei_device_remove() */
peck_dispatch_eis(peck);
with_server(peck) {
eis_client_disconnect(client);
@ -264,6 +260,9 @@ MUNIT_TEST(test_ei_disconnect_after_remove_after_received)
with_client(peck) {
ei_dispatch(ei);
_cleanup_ei_event_ struct ei_event *removed =
peck_ei_next_event(ei, EI_EVENT_DEVICE_REMOVED);
_cleanup_ei_event_ struct ei_event *disconnect =
peck_ei_next_event(ei, EI_EVENT_DISCONNECT);
}
@ -317,12 +316,12 @@ MUNIT_TEST(test_ei_device_basics)
return MUNIT_OK;
}
MUNIT_TEST(test_ei_device_add)
MUNIT_TEST(test_ei_device_add_remove)
{
_cleanup_peck_ struct peck *peck = peck_new();
with_server(peck) {
peck_enable_eis_behavior(peck, PECK_EIS_BEHAVIOR_ACCEPT_ALL);
peck_enable_eis_behavior(peck, PECK_EIS_BEHAVIOR_ACCEPT_CLIENT);
peck_dispatch_eis(peck);
}
@ -341,18 +340,126 @@ MUNIT_TEST(test_ei_device_add)
peck_dispatch_until_stable(peck);
_cleanup_eis_device_ struct eis_device *eis_device = NULL;
with_server(peck) {
_cleanup_eis_event_ struct eis_event *event =
peck_eis_next_event(eis, EIS_EVENT_DEVICE_ADDED);
eis_device = eis_device_ref(eis_event_get_device(event));
eis_device_allow_capability(eis_device, EIS_DEVICE_CAP_POINTER);
eis_device_connect(eis_device);
}
with_client(peck) {
peck_dispatch_ei(peck);
_cleanup_ei_event_ struct ei_event *event =
peck_ei_next_event(ei, EI_EVENT_DEVICE_ADDED);
struct ei_device *added = ei_event_get_device(event);
munit_assert_ptr_equal(device, added);
munit_assert(ei_device_has_capability(added, EI_DEVICE_CAP_POINTER));
ei_device_remove(device);
}
with_server(peck) {
peck_dispatch_eis(peck);
_cleanup_eis_event_ struct eis_event *event =
peck_eis_next_event(eis, EIS_EVENT_DEVICE_REMOVED);
munit_assert_ptr_equal(eis_event_get_device(event), eis_device);
eis_device_disconnect(eis_device);
}
with_client(peck) {
peck_dispatch_ei(peck);
_cleanup_ei_event_ struct ei_event *event =
peck_ei_next_event(ei, EI_EVENT_DEVICE_REMOVED);
struct ei_device *removed = ei_event_get_device(event);
munit_assert_ptr_equal(device, removed);
}
return MUNIT_OK;
}
MUNIT_TEST(test_ei_device_remove_forget_disconnect)
{
_cleanup_peck_ struct peck *peck = peck_new();
peck_enable_eis_behavior(peck, PECK_EIS_BEHAVIOR_ACCEPT_ALL);
peck_enable_ei_behavior(peck, PECK_EI_BEHAVIOR_AUTODEVICES);
peck_dispatch_until_stable(peck);
_cleanup_ei_device_ struct ei_device *d1 = NULL;
_cleanup_ei_device_ struct ei_device *d2 = NULL;
with_client(peck) {
d1 = ei_device_new(ei);
ei_device_configure_name(d1, "first");
ei_device_configure_capability(d1, EI_DEVICE_CAP_POINTER);
ei_device_add(d1);
d2 = ei_device_new(ei);
ei_device_configure_name(d2, "second");
ei_device_configure_capability(d2, EI_DEVICE_CAP_KEYBOARD);
ei_device_add(d2);
}
peck_dispatch_until_stable(peck);
peck_enable_eis_behavior(peck, PECK_EIS_BEHAVIOR_NONE);
with_client(peck) {
/* remove in order d1, d2 */
ei_device_remove(d1);
ei_device_remove(d2);
}
peck_dispatch_until_stable(peck);
struct eis_event *e1 = NULL;
with_server(peck) {
e1 = peck_eis_next_event(eis, EIS_EVENT_DEVICE_REMOVED);
_cleanup_eis_event_ struct eis_event *e2 =
peck_eis_next_event(eis, EIS_EVENT_DEVICE_REMOVED);
/* Explicitly disconnect device d2 */
eis_device_disconnect(eis_event_get_device(e2));
/* But forget to eis_device_disconnect(d1), happens
* automatically when the event is unref'd */
}
peck_dispatch_ei(peck);
with_client(peck) {
_cleanup_ei_event_ struct ei_event *event =
peck_ei_next_event(ei, EI_EVENT_DEVICE_REMOVED);
struct ei_device *removed = ei_event_get_device(event);
munit_assert_ptr_equal(d2, removed);
peck_assert_no_ei_events(ei);
}
/* unref should trigger eis_device_disconnect() */
eis_event_unref(e1);
peck_dispatch_ei(peck);
with_client(peck) {
_cleanup_ei_event_ struct ei_event *event =
peck_ei_next_event(ei, EI_EVENT_DEVICE_REMOVED);
struct ei_device *removed = ei_event_get_device(event);
munit_assert_ptr_equal(d1, removed);
peck_assert_no_ei_events(ei);
}
return MUNIT_OK;
}
MUNIT_TEST(test_ei_device_add_drop_caps)
{
_cleanup_peck_ struct peck *peck = peck_new();

10
test/test-eis.c
View file

@ -284,12 +284,14 @@ MUNIT_TEST(eistest_device_late_connect)
peck_dispatch_until_stable(peck);
with_client(peck) {
/* remove before server accepted */
ei_device_remove(ei_device);
}
peck_dispatch_until_stable(peck);
with_server(peck) {
/* accept even though the remove event is already pending */
eis_device_allow_capability(eis_device, EIS_DEVICE_CAP_POINTER);
eis_device_connect(eis_device);
}
@ -297,10 +299,18 @@ MUNIT_TEST(eistest_device_late_connect)
peck_dispatch_until_stable(peck);
with_server(peck) {
_cleanup_eis_event_ struct eis_event *removed =
peck_eis_next_event(eis, EIS_EVENT_DEVICE_REMOVED);
peck_assert_no_eis_events(eis);
}
peck_dispatch_until_stable(peck);
with_client(peck) {
_cleanup_ei_event_ struct ei_event *added =
peck_ei_next_event(ei, EI_EVENT_DEVICE_ADDED);
_cleanup_ei_event_ struct ei_event *removed =
peck_ei_next_event(ei, EI_EVENT_DEVICE_REMOVED);
peck_assert_no_ei_events(ei);
}