Bug 91872

Summary: libinput_unref() frees memory even when clients hold references to devices or seats
Product: Wayland Reporter: Peter Hutterer <peter.hutterer>
Component: libinputAssignee: Wayland bug list <wayland-bugs>
Status: RESOLVED WONTFIX QA Contact:
Severity: normal    
Priority: medium CC: peter.hutterer
Version: unspecified   
Hardware: Other   
OS: All   
Whiteboard:
i915 platform: i915 features:

Description Peter Hutterer 2015-09-04 03:46:30 UTC
The refcounting on the context itself does not take client-held references in the child structs into account.

Pseudocode example:
        li = libinput_path_create_context(&interface, NULL);
        device = libinput_path_add_device(li, "/dev/input/event0");
        libinput_device_ref(device);
        libinput_unref(li); <--- resources are cleaned up

The device has a refcount of at least 1 on cleanup, the client has access to the struct and could, in the future, try to call libinput_device_get_context().

However, the libinput context is cleaned up and all devices are removed. The device the client has a ref to points to a freed memory.

This also affects libinput_device_groups if the device is removed with a ref to the group still handling. It does not affect seats, since the context always has a ref to the seat and the bug simply isn't triggered.
Comment 1 Peter Hutterer 2015-09-04 04:24:35 UTC
The options for fixing this issue introduce incompatible behavior, so I'm closing this as a WONTFIX, this bug is mostly for archival purposes only.

If libinput_unref() changes to only do the work when all references are accounted for, callers have to release all existing references manually. This may cause a memory leak in existing callers.
In addition, the call to libinput_suspend() would not happen until the resources are released, causing wakeups on top of the memory leaks.

If libinput_unref() calls libinput_suspend() but leaves the memory until all references are released, the behaviour becomes unpredictable, and we get memory leaks in existing callers.
Requiring an explicit call to libinput_suspend() breaks existing callers.


So really, no good fix here and since the current behavior is also that of other libraries (e.g. libudev) we'll leave it as-is.

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.