ProcessTouchEvents() calls UDS for all touch events, but if the event type
was switched to TouchUpdate(pending end) UDS is a noop.
Daniel Drake found this can cause stuck buttons if a touch grab is
activated, rejected and the touch event is passed to a regular listener.
This sequence causes the TouchEnd to be changed to TouchUpdate(pending end).
The actual TouchEnd event is later generated by the server once it is passed
to the next listener. UDS is never called for this event, thus the button
remains logically down.
A previous patch suggested for UDS to handle TouchUpdate events [1], however
this would release the button when the first TouchEvent is processed, not
when the last grab has been released (as is the case for sync pointer
grabs). A client may thus have the grab on the device, receive a ButtonPress
but see the button logically up in an XQueryPointer request.
This patch adds a call to UDS to TouchEmitTouchEnd(). The device state must
be updated once a TouchEnd event was sent to the last grabbing listener and
the number of grabs on the touchpoint is 0.
[1] http://patchwork.freedesktop.org/patch/13464/
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
The cursor is referenced during CopyGrab(), thus doesn't need to be handled
manually anymore. It does need to be refcounted for temp grabs though.
The oldGrab handling in ProcGrabPointer is a leftover from the cursor in the
grab being refcounted, but the grab itself being a static struct in the
DeviceIntRec. Now that all grabs are copied, this lead to a double-free of
the cursor (Reproduced in Thunderbird, dragging an email twice (or more
often) causes a crash).
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Also add my name to the list of authors in COPYING
Reviewed-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
Signed-off-by: Keith Packard <keithp@keithp.com>
A client may call XIGrabDevice twice, overwriting the existing grab. Thus,
make sure we free the old copy after we copied it. Free it last, to make
sure our refcounts don't run to 0 and inadvertantly free something on the
way.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
If we have one listener left but it's not a grab, it cannot be in
LISTENER_HAS_ACCEPTED state.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
TouchListenerGone cleans up if a client disappears. Having this in
FreeGrab() triggers cyclic removal of grabs, emitting wrong events. In
particular, it would clean up a passive grab record while that grab is
active.
Move it to CloseDownClient() instead, cleaning up before we go.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Introduced in xorg-server-1.13.99.901-2-g9ad0fdb. Storing the grab pointer
in the listener turns out to be a bad idea. If the grab is not an active
grab or an implicit grab, the pointer stored is the one to the grab attached
on the window. This grab may be removed if the client calls UngrabButton or
similar while the touch is still active, leaving a dangling pointer.
To avoid this, copy the grab wherever we need to reference it later. This
is also what we do for pointer/keyboard grabs, where we copy the grab as
soon as it becomes active.
Reported-by: Maarten Lankhorst <maarten.lankhorst@canonical.com>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Obsolete since 4bc2761ad5. This struct
existed so copying a passive grab could be simply done by
activeGrab = *grab
and thus have a copy of the GrabPtr we'd get from various sources but still
be able to check device->grab for NULL.
Since 4bc2761 activeGrab is a pointer itself and points to the same memory
as grabinfo->grab, leaving us with the potential of dangling pointers if
either calls FreeGrab() and doesn't reset the other one.
There is no reader of activeGrab anyway, so simply removing it is
sufficient.
Note: field is merely renamed to keep the ABI. Should be removed in the
future.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Change the single if condition in the loop body to a
if (!foo) continue;
and re-indent the rest.
No functional changes.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
A sync grab is the owner once it gets events. If it doesn't replay the
event it will get all events from this touch, equivalent to accepting it.
If the touch has ended before XAllowEvents() is called, we also now need to
send the TouchEnd event and clean-up since we won't see anything more from
this touch.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
EmitTouchEnd calls DeliverTouchEvents directly instead of through
public.processInputProc. If a device is frozen, the TouchEnd is
processed while the device is waiting for a XAllowEvents and thus ends the
touch point (and the grab) before the client decided what to do with it. In
the case of ReplayPointer, this loses the event.
This is a hack, but making EmitTouchEnd use processInputProc breaks
approximately everything, especially the touch point is cleaned up during
ProcessTouchEvents. Working around that is a bigger hack than this.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
If a device is frozen in results to a grab, we need to enqueue the events.
This makes things complicated, and hard to follow since touch events are now
replayed in the history, pushed into EnqueueEvent, then replayed later
during PlayReleasedEvents in response to an XAllowEvents.
While the device is frozen, no touch events are processed, so if there is a
touch client with ownership mask _below_ the grab this will delay the
delivery and potentially screw gesture recognition. However, this is the
behaviour we have already anyway if the top-most client is a sync pgrab or
there is a sync grab active on the device when the TouchBegin was generated.
(also note, such a client would only reliably work in case of ReplayPointer
anyway)
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
If a touch is pending_finish and we just punted it to the next owner, that
client must receive a TouchEnd event.
If we just punted to the last owner and that owner not a touch grab, we need
to end the touch since this is the last event to be sent, and the client
cannot accept/reject this.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Delivering an event changes the state to LISTENER_IS_OWNER and we thus lose
the information of early acceptance.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
ActivateEarlyAccept() can only be called from a grabbing client, so we can
ignore the rest. And it's easy enough to get the client from that since
9ad0fdb135.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
If a TouchBegin is sent to a core client, that client is now the owner.
By the time the TouchEnd is being processed, the client cannot replay
anymore, so we can assume that this is the final touch end and we can clean
up the touch record.
Note: DeliverTouchEmulatedEvent is called for all listeners and immediately
bails out if the client is not the owner and thus shouldn't yet get the
event. Thus, check the return code.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
ef64b5ee97 (which introduced the
TOUCH_CLIENT_ID check) has a wrong assumption that generated touch events
(TOUCH_CLIENT_ID) should not terminate passive grabs.
This is untrue, a TouchEnd may be generated in response to a TouchReject
higher up. If we _deliver_ an event to a client, terminate the passive grab.
This requires us to count the actually delivered events too (first hunk).
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
If the device is currently grabbed as the result of a passive grab
activating, do not prepend that grab to the listeners (unlike active grabs).
Otherwise, a client with a passive pointer grab will prevent touch grabs
from activating higher up in the window stack.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
If we only have a single touch-grabbing client, setting the client as owner
would clean up the touch once the TouchEnd was processed. If the client then
calls XIAllowTouches() it will receive a BadValue for the touch ID (since
the internal record is already cleaned up).
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
If drivers supply incorrect values don't just quietly return False, spew to
the log so we can detect what's going on. All these cases are driver bugs
and should be fixed immediately.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Dave Airlie <airlied@redhat.com>
InitPointerClassDeviceStruct/InitKeyboardDeviceStruct allocate a
proximity/focus class, respectively. If a driver calls
InitFocusClassDeviceStruct or InitProximityClassDeviceStruct beforehand,
the previously allocated class is overwritten, leaking the memory.
Neither takes a parameter other than the device, so we can simply skip
initialising it if we already have one.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Dave Airlie <airlied@redhat.com>
==21860== 24 bytes in 1 blocks are still reachable in loss record 85 of 397
==21860== at 0x4C2B3F8: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==21860== by 0x61ED93: AllocateOutputBuffer (io.c:1037)
==21860== by 0x61E15A: WriteToClient (io.c:764)
==21860== by 0x457B30: ProcQueryExtension (extension.c:275)
==21860== by 0x43596B: Dispatch (dispatch.c:432)
==21860== by 0x425DAB: main (main.c:295)
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Dave Airlie <airlied@redhat.com>
If a client sends a request larger than maxBigRequestSize, the server is
supposed to ignore it.
Before commit cf88363d, the server would simply disconnect the client. After
that commit, it attempts to gracefully ignore the request by remembering how
long the client specified the request to be, and ignoring that many bytes.
However, if a client sends a BigReq header with a large size and disconnects
before actually sending the rest of the specified request, the server will
reuse the ConnectionInput buffer without resetting the ignoreBytes field. This
makes the server ignore new X clients' requests.
This fixes that behavior by resetting the ignoreBytes field when putting the
ConnectionInput buffer back on the FreeInputs list.
Signed-off-by: Robert Morell <rmorell@nvidia.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
DeleteCallbackManager() introduced for better symmetry in the caller, they
do the same thing.
==20085== 24 bytes in 1 blocks are still reachable in loss record 11 of 103
==20085== at 0x4C2A4CD: malloc (vg_replace_malloc.c:236)
==20085== by 0x43A097: CreateCallbackList (dixutils.c:837)
==20085== by 0x43A1D3: AddCallback (dixutils.c:869)
==20085== by 0x4B1736: GEExtensionInit (geext.c:209)
==20085== by 0x41C8A8: InitExtensions (miinitext.c:389)
==20085== by 0x5AC918: main (main.c:208)
==2042== 8 bytes in 1 blocks are still reachable in loss record 2 of 97
==2042== at 0x4C2A4CD: malloc (vg_replace_malloc.c:236)
==2042== by 0x4C2A657: realloc (vg_replace_malloc.c:525)
==2042== by 0x4802F5: XNFrealloc (utils.c:1095)
==2042== by 0x43A17A: CreateCallbackList (dixutils.c:855)
==2042== by 0x43A1EF: AddCallback (dixutils.c:870)
==2042== by 0x4B1752: GEExtensionInit (geext.c:209)
==2042== by 0x41C8A8: InitExtensions (miinitext.c:389)
==2042== by 0x5AC9E4: main (main.c:208)
==2042==
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Keith Packard <keithp@keithp.com>
==29423== 16 bytes in 1 blocks are definitely lost in loss record 73 of 328
==29423== at 0x4A06B6F: calloc (vg_replace_malloc.c:593)
==29423== by 0x5987C0: XIBarrierInit (xibarriers.c:908)
==29423== by 0x58F370: XInputExtensionInit (extinit.c:1300)
==29423== by 0x4F33C3: InitExtensions (miinitext.c:337)
==29423== by 0x4997DB: main (main.c:208)
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Keith Packard <keithp@keithp.com>
Backtrace logging etc. is already sigsafe, but the actual FatalError message
in response is not yet, leading to amusing logs like this:
(EE) Segmentation fault at address 0x0
(EE) BUG: triggered 'if (inSignalContext)'
(EE) BUG: log.c:499 in LogVMessageVerb()
(EE) Warning: attempting to log data in a signal unsafe manner while in
signal context.
Please update to check inSignalContext and/or use LogMessageVerbSigSafe() or
ErrorFSigSafe().
The offending log format message is:
Fatal server error:
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Keith Packard <keithp@keithp.com>
Mainly for %ld, smaller than int is propagated anyway, and %lld isn't really
used.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Keith Packard <keithp@keithp.com>
Heaps of these:
==2042== 15,360 bytes in 120 blocks are still reachable in loss record 94 of
97
==2042== at 0x4C2A4CD: malloc (vg_replace_malloc.c:236)
==2042== by 0x4C2A657: realloc (vg_replace_malloc.c:525)
==2042== by 0x45FB91: double_size (registry.c:65)
==2042== by 0x45FC97: RegisterRequestName (registry.c:85)
==2042== by 0x460095: RegisterExtensionNames (registry.c:179)
==2042== by 0x460729: dixResetRegistry (registry.c:334)
==2042== by 0x5AC992: main (main.c:201)
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Keith Packard <keithp@keithp.com>
==2547== 1 bytes in 1 blocks are still reachable in loss record 1 of 111
==2547== at 0x4C2A4CD: malloc (vg_replace_malloc.c:236)
==2547== by 0x64D1551: strdup (strdup.c:43)
==2547== by 0x4802FB: Xstrdup (utils.c:1113)
==2547== by 0x585B6C: XkbSetRulesUsed (xkbInit.c:219)
==2547== by 0x58700F: InitKeyboardDeviceStruct (xkbInit.c:595)
==2547== by 0x419FA3: vfbKeybdProc (InitInput.c:74)
==2547== by 0x425A3D: ActivateDevice (devices.c:540)
==2547== by 0x425F65: InitAndStartDevices (devices.c:713)
==2547== by 0x5ACA57: main (main.c:259)
and a few more of the above.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Keith Packard <keithp@keithp.com>
If both are missing, input device hotplugging will not work out of the box.
While we still have a DBus-API or the user may want to set AAD off all the
time, the most likely source of this is misconfiguration (i.e. lack of the
udev/hal devel packages).
Message printed last to make it more obvious to the user.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Keith Packard <keithp@keithp.com>
The only controls that still do something are DEVICE_RESOLUTION and
DEVICE_ENABLE.
XTest devices have no resolution to change, and they cannot be disabled. So
skip the lot, and prevent a crash in the DDX when it's trying to
de-reference pInfo->control_proc on device with no pInfo struct.
Likewise, don't allow setting device mode or the valuators.
XTest pointers are always relative, they don't have a mode.
Test cases:
xts5/XI/ChangeDeviceControl (1/10)
xts5/XI/SetDeviceValuators (1/6)
and a few others
Reported-by: Knut Petersen <Knut_Petersen@t-online.de>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Keith Packard <keithp@keithp.com>
Comment dates back to a pre-release version of XI2 that supported keysym
grabs. That never made it into a release, it was ditched before.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Keith Packard <keithp@keithp.com>
There's no point in turning on outputs connected to GPU screens during initial
configuration. Not only does this cause them to just display black, it also
confuses clients when these screens are attached to a master screen and RandR
reports that the outputs are already on.
Also, don't print the warning about no outputs being found on GPU screens,
since that's expected.
Signed-off-by: Aaron Plattner <aplattner@nvidia.com>
Reviewed-by: Dave Airlie <airlied@gmail.com>