Bug 81819

Summary:	Xwayland crashes when seat adds a capability and sends some input event
Product:	Wayland	Reporter:	Michael Forney <mforney>
Component:	XWayland	Assignee:	Wayland bug list <wayland-bugs>
Status:	RESOLVED FIXED	QA Contact:	Xorg Project Team <xorg-team>
Severity:	normal
Priority:	medium	CC:	cl91tp, mforney, stu_dby
Version:	unspecified
Hardware:	Other
OS:	All
Whiteboard:
i915 platform:		i915 features:
Attachments:	Patch fixing the issue Another patch

Description Michael Forney 2014-07-27 21:59:38 UTC

When Xwayland is started while the seat has limited capabilities (i.e. only pointer) and then later on more capabilities are added, any events sent from those new devices cause a crash.

This seems to be because in xwayland-input.c:add_device where the devices are created, xwayland adds them with AddInputDevice, but does not activate or enable them (with ActivateDevice and EnableDevice), so miPointerDeviceInitialize is never called on these devices.

The devices with the capabilities present when Xwayland starts do not have this problem because they are activated and enabled in InitAndStartDevices.

I think adding a call to ActivateDevice, followed by a call to EnableDevice at the end of add_device should be enough (and it does solve the problem), but I am not familiar enough with the X server architecture to know if this is the right solution.

Comment 1 Michael Forney 2014-07-27 22:22:07 UTC

This can be reproduced by simply starting an X application, unplugging all pointing devices, then plugging one in and moving the cursor into the X window.

Comment 2 Michael Forney 2014-07-27 22:23:49 UTC

Created attachment 103567 [details] [review]
Patch fixing the issue

Comment 3 Boyan Ding 2014-07-28 13:18:36 UTC

Does http://cgit.freedesktop.org/xorg/xserver/commit/?id=2172714c67d8701aa54c202e89f246f1dddac80a fix it for you?

Comment 4 Michael Forney 2014-07-29 01:09:46 UTC

Partially.

The commit you referenced fixes the case where a seat capability disappears and then reappears (such as when disconnecting then reconnecting a mouse, or switching VTs).

However, if Xwayland is started without some capability (for example, no pointing devices connected), then a mouse is plugged in, the problem remains. This is can be reproduced by starting weston with no pointing devices, switching to another VT, launching weston-terminal (by setting WAYLAND_DISPLAY), switching back to the weston VT, launching an X client in the opened terminal (by setting DISPLAY), then plugging in a mouse and moving the cursor over the X client.

The issue is that the devices created after Xwayland startup are never activated.

Comment 5 Boyan Ding 2014-08-02 01:57:43 UTC

Created attachment 103842 [details] [review]
Another patch

Actually I sent a patch similar to yours to xorg-devel a month ago, but another patch (the commit I mentioned above) was accepted, clearly people only thought about switching VTs and didn't notice the problem you described.

Now with that commit in mind, I propose this patch. Is it okay?

Comment 6 Michael Forney 2014-08-03 04:56:40 UTC

(In reply to comment #5)
> Created attachment 103842 [details] [review] [review]
> Another patch
> 
> Actually I sent a patch similar to yours to xorg-devel a month ago, but
> another patch (the commit I mentioned above) was accepted, clearly people
> only thought about switching VTs and didn't notice the problem you described.
> 
> Now with that commit in mind, I propose this patch. Is it okay?

Yep. I can confirm that your patch (with a small typo fix) fixes the issue as well.

Thanks!

Comment 7 Chang Liu 2014-12-10 22:50:02 UTC

Can confirm that this patch (with a typo fix) fixes Xwayland crashing when switching tty.

Now if anyone could get this patch upstream...

Comment 8 Pekka Paalanen 2014-12-11 12:49:36 UTC

If the patch is not already upstream, you would better not mark the bug as fixed then. Since it's not fixed upstream. :-)

Marking a bug fixed just makes developers not see it anymore.

Reopening, assuming the patch isn't upstream. Could you poke xorg-devel@, please?

Comment 9 Daniel Stone 2014-12-11 13:33:47 UTC

Please post it with the following line:
Reviewed-by: Daniel Stone <daniels@collabora.com>

as well as Cc'ing keithp@keithp.com, asking him to pull it for 1.17.

Comment 10 Boyan Ding 2014-12-11 14:26:38 UTC

The patch is at http://lists.x.org/archives/xorg-devel/2014-August/043432.html. Should I send a ping?

Comment 11 Chang Liu 2014-12-11 18:40:53 UTC

(In reply to Boyan Ding from comment #10)
> The patch is at
> http://lists.x.org/archives/xorg-devel/2014-August/043432.html. Should I
> send a ping?

Yes please. Also is there any chance that this patch can be backported to 1.16 (and be included in say, the 1.16.3 release)? It's going to be a while before 1.17 can be released. It will be even longer before it can make into most distros. Considering that this is purely a bug fix that has minimal impact on the rest of the codebase I'd say it is fairly reasonable to do so.

Comment 12 Olivier Fourdan 2015-09-22 09:54:22 UTC

Patch has been merged in master.

http://cgit.freedesktop.org/xorg/xserver/commit/?id=1ba4fde

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.