[PATCH libinput v2 2/3] Make context reference counted
Jonas Ådahl
jadahl at gmail.com
Tue Jun 24 15:06:58 PDT 2014
Instead of only allowing one owner keeping a libinput context alive,
make context reference counted, replacing libinput_destroy() with
libinput_unref() while adding another function libinput_ref().
Even though there might not be any current use cases, it doesn't mean we
should hard code this usage model in the API. The old behaviour can be
emulated by never calling libinput_ref() while replacing
libinput_destroy() with libinput_unref().
Signed-off-by: Jonas Ådahl <jadahl at gmail.com>
---
Changes since v2:
_ref() returns the passed context.
_unref() returns NULL when context is destroyed, otherwise the passed
context.
Previous thread inlined:
On Tue, Jun 24, 2014 at 11:29:37AM +1000, Peter Hutterer wrote:
> On Tue, Jun 24, 2014 at 01:20:47AM +0300, Ran Benita wrote:
> > On Mon, Jun 23, 2014 at 11:56:41PM +0200, Jonas Ådahl wrote:
> > >
> > > There are no users I know of which currently would benefit from this but
> > > I'm thinking it might be better to at least not disallow. Yay or nay?
>
> The Xorg libinput driver is that use-case. It shares the libinput context
> between separate modules and I did have a patchset to refcount the libinput
> context at some point but ditched it.
>
> The specific details here are: the driver module has a static libinput
> context, initialized with the first device. All devices are then
> added to/removed from that context. ref/unref works fine until you remove
> the last device - then the context becomes invalid but you don't know, and
> the next time you add a device things blow up.
>
> So this patchset doesn't really help, I'd still need to refcount manually
> to figure out when the context was destroyed. With Ran's suggestion the
> story is a bit different: if we make ref() return the object and unref()
> return the object or NULL if it was destroyed, then I can use this in the
> xorg driver. [and for consistency that would have to apply to all other
> ref/unref() functions too.]o
Making ref/unref() return the resulting pointer context sounds better
returning nothing, don't see a reason not making that the case for the
existing ref/unref() actually, so sent a separate patch is this new
series making that change.
>
> There's one more problem:
> libinput_destroy() is an explicit "kill everthing", making all objects
> invalid.
> libinput_unref() doesn't, objects stay valid or not and without knowing the
> refcount you don't really know. And as your patch says: "It is up to the
> caller to keep track of the number of references held". I wonder what use
> the refcounting is then if we rely on the caller to track it anyway?
If some code relies on objects from a libinput context staying alive,
then that code needs to belong to some module that owns a reference,
but for the use case is that the context is simply there to be
potentially reused, then the ref/unref() return value change would solve
that problem.
>
> So I'm a bit meh on this: it changes the semantics for what I can tell is of
> no benefit right now, and the benefits that could come from it aren't
> available in this API.
So, this version has the changes you and Ran pointed out and solves the
issue with the X.org driver use case. The two use cases (two units of a
program with independent lifetime; shared context at a higher level)
should work.
Jonas
src/libinput-private.h | 1 +
src/libinput.c | 21 ++++++++++++++++++---
src/libinput.h | 28 ++++++++++++++++++++++++----
src/udev-seat.c | 2 +-
test/keyboard.c | 2 +-
test/litest.c | 2 +-
test/log.c | 10 +++++-----
test/misc.c | 10 +++++-----
test/path.c | 20 ++++++++++----------
test/pointer.c | 2 +-
test/udev.c | 16 ++++++++--------
tools/event-debug.c | 4 ++--
tools/event-gui.c | 2 +-
13 files changed, 78 insertions(+), 42 deletions(-)
diff --git a/src/libinput-private.h b/src/libinput-private.h
index f0bda1f..cfe6535 100644
--- a/src/libinput-private.h
+++ b/src/libinput-private.h
@@ -57,6 +57,7 @@ struct libinput {
const struct libinput_interface *interface;
const struct libinput_interface_backend *interface_backend;
void *user_data;
+ int refcount;
};
typedef void (*libinput_seat_destroy_func) (struct libinput_seat *seat);
diff --git a/src/libinput.c b/src/libinput.c
index d4d5711..3aeca7d 100644
--- a/src/libinput.c
+++ b/src/libinput.c
@@ -502,6 +502,7 @@ libinput_init(struct libinput *libinput,
libinput->interface = interface;
libinput->interface_backend = interface_backend;
libinput->user_data = user_data;
+ libinput->refcount = 1;
list_init(&libinput->source_destroy_list);
list_init(&libinput->seat_list);
@@ -530,15 +531,27 @@ libinput_drop_destroyed_sources(struct libinput *libinput)
list_init(&libinput->source_destroy_list);
}
-LIBINPUT_EXPORT void
-libinput_destroy(struct libinput *libinput)
+LIBINPUT_EXPORT struct libinput *
+libinput_ref(struct libinput *libinput)
+{
+ libinput->refcount++;
+ return libinput;
+}
+
+LIBINPUT_EXPORT struct libinput *
+libinput_unref(struct libinput *libinput)
{
struct libinput_event *event;
struct libinput_device *device, *next_device;
struct libinput_seat *seat, *next_seat;
if (libinput == NULL)
- return;
+ return NULL;
+
+ assert(libinput->refcount > 0);
+ libinput->refcount--;
+ if (libinput->refcount > 0)
+ return libinput;
libinput_suspend(libinput);
@@ -562,6 +575,8 @@ libinput_destroy(struct libinput *libinput)
libinput_drop_destroyed_sources(libinput);
close(libinput->epoll_fd);
free(libinput);
+
+ return NULL;
}
LIBINPUT_EXPORT void
diff --git a/src/libinput.h b/src/libinput.h
index 3503b76..72375f7 100644
--- a/src/libinput.h
+++ b/src/libinput.h
@@ -793,6 +793,9 @@ struct libinput_interface {
* device are ignored. Such devices and those that failed to open
* ignored until the next call to libinput_resume().
*
+ * The reference count of the context is initialized to 1. See @ref
+ * libinput_unref.
+ *
* @param interface The callback interface
* @param user_data Caller-specific data passed to the various callback
* interfaces.
@@ -818,6 +821,9 @@ libinput_udev_create_for_seat(const struct libinput_interface *interface,
* The context is fully initialized but will not generate events until at
* least one device has been added.
*
+ * The reference count of the context is initialized to 1. See @ref
+ * libinput_unref.
+ *
* @param interface The callback interface
* @param user_data Caller-specific data passed to the various callback
* interfaces.
@@ -967,13 +973,27 @@ libinput_suspend(struct libinput *libinput);
/**
* @ingroup base
*
- * Destroy the libinput context. After this, object references associated with
- * the destroyed context are invalid and may not be interacted with.
+ * Add a reference to the context. A context is destroyed whenever the
+ * reference count reaches 0. See @ref libinput_unref.
+ *
+ * @param libinput A previously initialized valid libinput context
+ * @return The passed libinput context
+ */
+struct libinput *
+libinput_ref(struct libinput *libinput);
+
+/**
+ * @ingroup base
+ *
+ * Dereference the libinput context. After this, the context may have been
+ * destroyed, if the last reference was dereferenced. If so, the context is
+ * invalid and may not be interacted with.
*
* @param libinput A previously initialized libinput context
+ * @return NULL if context was destroyed otherwise the passed context
*/
-void
-libinput_destroy(struct libinput *libinput);
+struct libinput *
+libinput_unref(struct libinput *libinput);
/**
* @ingroup base
diff --git a/src/udev-seat.c b/src/udev-seat.c
index 38a13b7..5b61dee 100644
--- a/src/udev-seat.c
+++ b/src/udev-seat.c
@@ -358,7 +358,7 @@ libinput_udev_create_for_seat(const struct libinput_interface *interface,
if (udev_input_enable(&input->base) < 0) {
udev_unref(udev);
- libinput_destroy(&input->base);
+ libinput_unref(&input->base);
free(input);
return NULL;
}
diff --git a/test/keyboard.c b/test/keyboard.c
index a518b66..83153bb 100644
--- a/test/keyboard.c
+++ b/test/keyboard.c
@@ -108,7 +108,7 @@ START_TEST(keyboard_seat_key_count)
for (i = 0; i < num_devices; ++i)
litest_delete_device(devices[i]);
- libinput_destroy(libinput);
+ libinput_unref(libinput);
}
END_TEST
diff --git a/test/litest.c b/test/litest.c
index 0a9cc72..55ba678 100644
--- a/test/litest.c
+++ b/test/litest.c
@@ -573,7 +573,7 @@ litest_delete_device(struct litest_device *d)
libinput_device_unref(d->libinput_device);
if (d->owns_context)
- libinput_destroy(d->libinput);
+ libinput_unref(d->libinput);
libevdev_free(d->evdev);
libevdev_uinput_destroy(d->uinput);
memset(d,0, sizeof(*d));
diff --git a/test/log.c b/test/log.c
index a281820..fe67d68 100644
--- a/test/log.c
+++ b/test/log.c
@@ -86,7 +86,7 @@ START_TEST(log_handler_invoked)
ck_assert_int_gt(log_handler_called, 0);
log_handler_called = 0;
- libinput_destroy(li);
+ libinput_unref(li);
libinput_log_set_priority(pri);
}
END_TEST
@@ -106,7 +106,7 @@ START_TEST(log_userdata_NULL)
ck_assert_int_gt(log_handler_called, 0);
log_handler_called = 0;
- libinput_destroy(li);
+ libinput_unref(li);
libinput_log_set_priority(pri);
}
@@ -127,7 +127,7 @@ START_TEST(log_userdata)
ck_assert_int_gt(log_handler_called, 0);
log_handler_called = 0;
- libinput_destroy(li);
+ libinput_unref(li);
libinput_log_set_priority(pri);
}
END_TEST
@@ -148,7 +148,7 @@ START_TEST(log_handler_NULL)
log_handler_called = 0;
libinput_log_set_handler(simple_log_handler, NULL);
- libinput_destroy(li);
+ libinput_unref(li);
libinput_log_set_priority(pri);
}
END_TEST
@@ -173,7 +173,7 @@ START_TEST(log_priority)
log_handler_called = 0;
- libinput_destroy(li);
+ libinput_unref(li);
libinput_log_set_priority(pri);
}
END_TEST
diff --git a/test/misc.c b/test/misc.c
index 133bdb6..ad2e1f6 100644
--- a/test/misc.c
+++ b/test/misc.c
@@ -133,7 +133,7 @@ START_TEST(event_conversion_device_notify)
libinput_event_destroy(event);
}
- libinput_destroy(li);
+ libinput_unref(li);
libevdev_uinput_destroy(uinput);
ck_assert_int_gt(device_added, 0);
@@ -194,7 +194,7 @@ START_TEST(event_conversion_pointer)
libinput_event_destroy(event);
}
- libinput_destroy(li);
+ libinput_unref(li);
libevdev_uinput_destroy(uinput);
ck_assert_int_gt(motion, 0);
@@ -254,7 +254,7 @@ START_TEST(event_conversion_pointer_abs)
libinput_event_destroy(event);
}
- libinput_destroy(li);
+ libinput_unref(li);
libevdev_uinput_destroy(uinput);
ck_assert_int_gt(motion, 0);
@@ -304,7 +304,7 @@ START_TEST(event_conversion_key)
libinput_event_destroy(event);
}
- libinput_destroy(li);
+ libinput_unref(li);
libevdev_uinput_destroy(uinput);
ck_assert_int_gt(key, 0);
@@ -364,7 +364,7 @@ START_TEST(event_conversion_touch)
libinput_event_destroy(event);
}
- libinput_destroy(li);
+ libinput_unref(li);
libevdev_uinput_destroy(uinput);
ck_assert_int_gt(touch, 0);
diff --git a/test/path.c b/test/path.c
index 24f60e0..99b474e 100644
--- a/test/path.c
+++ b/test/path.c
@@ -65,7 +65,7 @@ START_TEST(path_create_NULL)
ck_assert(li == NULL);
li = libinput_path_create_context(&simple_interface, NULL);
ck_assert(li != NULL);
- libinput_destroy(li);
+ libinput_unref(li);
ck_assert_int_eq(open_func_count, 0);
ck_assert_int_eq(close_func_count, 0);
@@ -92,7 +92,7 @@ START_TEST(path_create_invalid)
ck_assert_int_eq(open_func_count, 0);
ck_assert_int_eq(close_func_count, 0);
- libinput_destroy(li);
+ libinput_unref(li);
ck_assert_int_eq(close_func_count, 0);
open_func_count = 0;
@@ -126,7 +126,7 @@ START_TEST(path_create_destroy)
ck_assert_int_eq(open_func_count, 1);
libevdev_uinput_destroy(uinput);
- libinput_destroy(li);
+ libinput_unref(li);
ck_assert_int_eq(close_func_count, 1);
open_func_count = 0;
@@ -372,7 +372,7 @@ START_TEST(path_suspend)
libinput_resume(li);
libevdev_uinput_destroy(uinput);
- libinput_destroy(li);
+ libinput_unref(li);
open_func_count = 0;
close_func_count = 0;
@@ -406,7 +406,7 @@ START_TEST(path_double_suspend)
libinput_resume(li);
libevdev_uinput_destroy(uinput);
- libinput_destroy(li);
+ libinput_unref(li);
open_func_count = 0;
close_func_count = 0;
@@ -440,7 +440,7 @@ START_TEST(path_double_resume)
libinput_resume(li);
libevdev_uinput_destroy(uinput);
- libinput_destroy(li);
+ libinput_unref(li);
open_func_count = 0;
close_func_count = 0;
@@ -523,7 +523,7 @@ START_TEST(path_add_device_suspend_resume)
libevdev_uinput_destroy(uinput1);
libevdev_uinput_destroy(uinput2);
- libinput_destroy(li);
+ libinput_unref(li);
open_func_count = 0;
close_func_count = 0;
@@ -614,7 +614,7 @@ START_TEST(path_add_device_suspend_resume_fail)
ck_assert_int_eq(nevents, 2);
libevdev_uinput_destroy(uinput2);
- libinput_destroy(li);
+ libinput_unref(li);
open_func_count = 0;
close_func_count = 0;
@@ -704,7 +704,7 @@ START_TEST(path_add_device_suspend_resume_remove_device)
ck_assert_int_eq(nevents, 1);
libevdev_uinput_destroy(uinput1);
- libinput_destroy(li);
+ libinput_unref(li);
open_func_count = 0;
close_func_count = 0;
@@ -790,7 +790,7 @@ START_TEST(path_seat_recycle)
ck_assert(found == 1);
- libinput_destroy(li);
+ libinput_unref(li);
libevdev_uinput_destroy(uinput);
}
diff --git a/test/pointer.c b/test/pointer.c
index 346e59b..7d5668f 100644
--- a/test/pointer.c
+++ b/test/pointer.c
@@ -292,7 +292,7 @@ START_TEST(pointer_seat_button_count)
for (i = 0; i < num_devices; ++i)
litest_delete_device(devices[i]);
- libinput_destroy(libinput);
+ libinput_unref(libinput);
}
END_TEST
diff --git a/test/udev.c b/test/udev.c
index 6af2cb0..09c2a94 100644
--- a/test/udev.c
+++ b/test/udev.c
@@ -97,7 +97,7 @@ START_TEST(udev_create_seat0)
ck_assert(event != NULL);
libinput_event_destroy(event);
- libinput_destroy(li);
+ libinput_unref(li);
udev_unref(udev);
}
END_TEST
@@ -124,7 +124,7 @@ START_TEST(udev_create_empty_seat)
ck_assert(event == NULL);
libinput_event_destroy(event);
- libinput_destroy(li);
+ libinput_unref(li);
udev_unref(udev);
}
END_TEST
@@ -169,7 +169,7 @@ START_TEST(udev_added_seat_default)
ck_assert(default_seat_found);
- libinput_destroy(li);
+ libinput_unref(li);
udev_unref(udev);
}
END_TEST
@@ -200,7 +200,7 @@ START_TEST(udev_double_suspend)
libinput_resume(li);
libinput_event_destroy(event);
- libinput_destroy(li);
+ libinput_unref(li);
udev_unref(udev);
}
END_TEST
@@ -231,7 +231,7 @@ START_TEST(udev_double_resume)
libinput_resume(li);
libinput_event_destroy(event);
- libinput_destroy(li);
+ libinput_unref(li);
udev_unref(udev);
}
END_TEST
@@ -289,7 +289,7 @@ START_TEST(udev_suspend_resume)
process_events_count_devices(li, &num_devices);
ck_assert_int_gt(num_devices, 0);
- libinput_destroy(li);
+ libinput_unref(li);
udev_unref(udev);
}
END_TEST
@@ -322,7 +322,7 @@ START_TEST(udev_device_sysname)
libinput_event_destroy(ev);
}
- libinput_destroy(li);
+ libinput_unref(li);
udev_unref(udev);
}
END_TEST
@@ -396,7 +396,7 @@ START_TEST(udev_seat_recycle)
ck_assert(found == 1);
- libinput_destroy(li);
+ libinput_unref(li);
udev_unref(udev);
}
END_TEST
diff --git a/tools/event-debug.c b/tools/event-debug.c
index 34acfce..95e7628 100644
--- a/tools/event-debug.c
+++ b/tools/event-debug.c
@@ -163,7 +163,7 @@ open_device(struct libinput **li, const char *path)
device = libinput_path_add_device(*li, path);
if (!device) {
fprintf(stderr, "Failed to initialized device %s\n", path);
- libinput_destroy(*li);
+ libinput_unref(*li);
return 1;
}
@@ -478,7 +478,7 @@ main(int argc, char **argv)
mainloop(li);
- libinput_destroy(li);
+ libinput_unref(li);
if (udev)
udev_unref(udev);
diff --git a/tools/event-gui.c b/tools/event-gui.c
index e080ea8..d2b0428 100644
--- a/tools/event-gui.c
+++ b/tools/event-gui.c
@@ -467,7 +467,7 @@ main(int argc, char *argv[])
gtk_main();
- libinput_destroy(li);
+ libinput_unref(li);
udev_unref(udev);
return 0;
--
1.9.1
More information about the wayland-devel
mailing list