Age | Commit message (Collapse) | Author |
|
Ensure that when calculating the size of the incoming response from the
Xserver, we don't overflow the integer used in the calculations when we
multiply the int32_t length by 4 and add it to the default response size.
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
(cherry picked from commit 1b33867fa996034deb50819ae54640be501f8d20)
|
|
Allows configure to set defines such as _POSIX_SOURCE in config.h
that affect functions exposed by system headers and get consistent
results across all the source files.
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
|
|
On FreeBSD MSG_WAITALL on a non-blocking socket fails immediately if less bytes
than were asked for are available. This is different than the behavior on linux
where as many bytes as are available are returned in this case. Other OS
apparently follow the FreeBSD behavior.
_xcb_in_read() is used to fill xcb's read buffer, thus this function will call
recv() with a big length argument (xcb's read buffer is by default 16 KiB
large). That many bytes are highly unlikely to be available in the kernel
buffer.
This means that _xcb_in_read() always failed on FreeBSD. Since the socket was
still signaled as readable by poll(), this bug even resulted in a busy loop.
The same issue is present in read_block(), but here it is slightly different.
read_block() is called when we read the first few bytes of an event or a reply,
so that we already know its length. This means that we should be able to use
MSG_WAITALL here, because we know how many bytes there have to be.
However, that function could busy loop, too, when only the first few bytes of
the packet were sent while the rest is stuck somewhere on the way to us. Thus,
MSG_WAITALL should be removed here, too.
Thanks to Christoph Egger from Debian for noticing the problem, doing all the
necessary debugging and figuring out what the problem was! This patch is 99%
from debian. Thanks for all the work.
This bug was introduced in commit 2dcf8b025be88a25d4333abdc28d425b88238d96.
This commit also reverts commit 9061ee45b8dbe5431c23e3f628089d703ccad0b1.
Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=45776
Signed-off-by: Uli Schlachter <psychon@znc.in>
Reviewed-by: Josh Triplett <josh@joshtriplett.org>
|
|
2dcf8b025be88a25d4333abdc28d425b88238d96 was causing some regressions on
darwin, so go back to using read(2) there until I have time to investigate
further.
Signed-off-by: Jeremy Huddleston <jeremyhu@apple.com>
|
|
Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=41443
Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=42304
I have added more xcb connection error states at xcb.h header.
Also I have removed global error_connection variable, and added
an interface that returns connection error state.
TBD:
I will segregate errors states in a separate header file and try to
provide more precise error states, in future. Also I will give patch
for libX11, in that patch xcb_connection_t::has_error will be passed
to default io handler of libX11. This value can then be used for
displaying error messages.
Reviewed-by: Rami Ylimäki <rami.ylimaki@vincit.fi>
Reviewed-by: Uli Schlachter <psychon@znc.in>
Signed-off-by: Arvind Umrao <arvind.umrao@oracle.com>
|
|
Imagine two threads:
Thread#1: for(;;) { xcb_get_input_focus_reply(c, xcb_get_input_focus(c), 0); }
Thread#2: for(;;) { xcb_poll_for_event(c); }
Since xcb_poll_for_event() calls _xcb_in_read() directly without synchronizing
with any other readers, this causes two threads to end up calling recv() at the
same time. We now have a race because any of these two threads could get read
the GetInputFocus reply.
If thread#2 reads this reply, it will be put in the appropriate queue and
thread#1 will still be stuck in recv(), although its reply was already received.
If no other reply or event causes this thread to wake up, the process deadlocks.
To fix this, we have to make sure that there is only ever one thread reading
from the connection. The obvious solution is to check in poll_for_next_event()
if another thread is already reading (in which case c->in.reading != 0) and not
to read from the wire in this case.
This solution is actually correct if we assume that the other thread is blocked
in poll() which means there isn't any data which can be read. Since we already
checked that there is no event in the queue this means that
poll_for_next_event() didn't find any event to return.
There might be a small race here where the other thread already determined that
there is data to read, but it still has to wait for c->iolock. However, this
means that the next poll_for_next_event() will be able to read the event, so
this shouldn't cause any problems.
Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=40372
Signed-off-by: Uli Schlachter <psychon@znc.in>
Signed-off-by: Peter Harris <pharris@opentext.com>
|
|
wire-order."
This function was intended to allow libX11 to fix a multi-threaded hang,
but the corresponding libX11 patch caused single-threaded apps to spin
sometimes. Since I've retracted that patch, this patch has no users and
shouldn't go into a release unless/until that changes.
This reverts commit 2415c11dec5e5adb0c17f98aa52fbb371a4f8f23.
Conflicts:
src/xcb.h
src/xcb_in.c
Signed-off-by: Jamey Sharp <jamey@minilop.net>
|
|
In some circumstances using xcb_poll_for_event is suboptimal because
it checks the connection for new events. This may lead to a lot of
failed nonblocking read system calls.
Signed-off-by: Rami Ylimäki <rami.ylimaki@vincit.fi>
Signed-off-by: Jamey Sharp <jamey@minilop.net>
|
|
This patch is necessary so xcb reads the payload after the message
for GenericEvents with the 0x80 flag turned on.
Signed-off-by: Carlos Garnacho <carlosg@gnome.org>
Signed-off-by: Jamey Sharp <jamey@minilop.net>
|
|
Signed-off-by: Jamey Sharp <jamey@minilop.net>
Reviewed-by: Josh Triplett <josh@freedesktop.org>
|
|
Signed-off-by: Jamey Sharp <jamey@minilop.net>
Reviewed-by: Josh Triplett <josh@freedesktop.org>
|
|
Later patches will insert reader_list entries from other entry points.
Signed-off-by: Jamey Sharp <jamey@minilop.net>
Reviewed-by: Josh Triplett <josh@freedesktop.org>
|
|
It's possible to call xcb_wait_for_reply more than once for a single
request. In this case we are nice and let reply waiters continue so
that they can notice that the reply is not available
anymore. Otherwise an event waiter could just signal the reply waiter
that got its reply to continue but leave a waiter for an earlier reply
blocked.
Below is an example sequence for reproducing this problem.
thread #1 (XNextEvent)
- waits for events
thread #2 (XSync)
- executes request #2
- waits for reply #2
thread #1
- reads reply #2
- signals waiter of reply #2 to continue
- waits for events
thread #2
- handles reply #2
thread #3 (XCloseDisplay)
- executes request #3
- waits for reply #2
thread #1
- reads reply #3
- nobody is waiting for reply #3 so don't signal
- wait for events
Of course it may be questionable to wait for a reply twice, but XCB
should be smart enough to let clients continue if they choose to do
so.
Signed-off-by: Rami Ylimäki <rami.ylimaki@vincit.fi>
Signed-off-by: Jamey Sharp <jamey@minilop.net>
|
|
Widen sequence numbers on entry to those public APIs that still take
32-bit sequence numbers.
Signed-off-by: Jamey Sharp <jamey@minilop.net>
|
|
If you discard a sequence number that has multiple responses already
read, this will do more allocations than necessary. But nobody cares
about ListFontsWithInfo.
Signed-off-by: Jamey Sharp <jamey@minilop.net>
|
|
Signed-off-by: Jamey Sharp <jamey@minilop.net>
|
|
Conflicts:
src/xcb_conn.c
src/xcb_util.c
Signed-off-by: Peter Harris <pharris@opentext.com>
|
|
This fixes the test case I have so far for Havoc's report that
xcb_request_check hangs.
Rationale: Since we have a void cookie, request_expected can't have been
set equal to this sequence number when the request was sent; it can only
have become equal due to the arrival of an event or error. If it became
equal due to an event then we still need to sync. If it became equal due
to an error, then request_completed will have been updated, which means
we correctly won't sync.
Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=29599
However, Havoc reports that he can still reproduce the problem, so we
may be revisiting this later.
Reported-by: Havoc Pennington <hp@pobox.com>
Signed-off-by: Jamey Sharp <jamey@minilop.net>
|
|
platforms. MSG_WAITALL is undefined in MinGW so it's been explicitly
defined in xcb_in.c
|
|
do..while loop execute only once. Also set the return value to -1 in
_xcb_open if control reaches the end - if all goes well it shouldn't
reach there.
|
|
Since writers must make sure they read as well, threads may have gone to
sleep waiting for the opportunity to read. The writer must wake up one
of those readers or the application can hang.
Signed-off-by: Jamey Sharp <jamey@minilop.net>
Reviewed-by: Josh Triplett <josh@freedesktop.org>
|
|
xcb_ge_event_t has its length field in the same place that
xcb_generic_reply_t does, so there's no need to cast the generic reply.
Signed-off-by: Jamey Sharp <jamey@minilop.net>
Cc: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Julien Danjou <julien@danjou.info>
|
|
|
|
that.Replaced one instance ofWIN32 with _WIN32 in each xcb_in.c and xcb_conn.c
|
|
|
|
This function is useful for dynamic language garbage collectors. Frequently
a GC cycle may run before you want to block wainting for a reply.
This function is also marginally useful for libxcb apps that issue
speculative requests (eg. xlsclients).
Reviewed-by: Jamey Sharp <jamey@minilop.net>
Tested-by: Eamon Walsh <efw@eamonwalsh.com>
Signed-off-by: Peter Harris <pharris@opentext.com>
|
|
Signed-off-by: Julien Danjou <julien@danjou.info>
|
|
Libraries like Xlib, some XCB language bindings, and potentially others
have a common problem: they want to share the X connection with XCB. This
requires coordination of request sequence numbers. Previously, XCB had an
Xlib-specific lock, and allowed Xlib to block XCB from making requests.
Now we've replaced that lock with a handoff mechanism, xcb_take_socket,
allowing external code to ask XCB for permission to take over the write
side of the socket and send raw data with xcb_writev. The caller of
xcb_take_socket must supply a callback which XCB can call when it wants
the write side of the socket back to make a request. This callback
synchronizes with the external socket owner, flushes any output queues if
appropriate, and then returns the sequence number of the last request sent
over the socket.
Commit by Josh Triplett and Jamey Sharp.
Handoff mechanism inspired by Keith Packard.
|
|
External APIs that used 32-bit sequence numbers continue to do so.
Commit by Josh Triplett and Jamey Sharp.
|
|
This allows optimizing adjacent pending replies with the same flags, and
will help support default flags for a range of future requests.
Commit by Josh Triplett and Jamey Sharp.
|
|
These functions are once again a single pthread call, so just make that
call directly.
|
|
NULL so it's very rare)
Signed-off-by: Julien Danjou <julien@danjou.info>
|
|
Signed-off-by: Julien Danjou <julien@danjou.info>
|
|
|
|
GenericEvent can be more than 32 bytes long. Ensure that the required data is
pulled off the wire and tack it onto the event.
Due to the structure of the xcb_generic_event_t, the data is appended AFTER
the full_sequence field.
|
|
|
|
The initial implementation of Plan 7 dumped all X errors into the event
queue, because the record of a pending reply was pruned too early if an
error occurred in place of the expected reply.
|
|
|
|
|
|
|
|
|
|
Rename API to follow a new naming convention:
* XCB_CONSTANTS_UPPERCASE_WITH_UNDERSCORES
* xcb_functions_lowercase_with_underscores
* xcb_types_lowercase_with_underscores_and_suffix_t
* expand all abbreviations like "req", "rep", and "iter"
Word boundaries for the names in the protocol descriptions fall:
* Wherever the protocol descriptions already have an underscore
* Between a lowercase letter and a subsequent uppercase letter
* Before the last uppercase letter in a string of uppercase letters followed
by a lowercase letter (such as in LSBFirst between LSB and First)
* Before and after a string of digits (with exceptions for sized types like
xcb_char2b_t and xcb_glx_float32_t to match the stdint.h convention)
Also fix up some particular naming issues:
* Rename shape_op and shape_kind to drop the "shape_" prefix, since otherwise
these types end up as xcb_shape_shape_{op,kind}_t.
* Remove leading underscores from enums in the GLX protocol description,
previously needed to ensure a word separator, but now redundant.
This renaming breaks code written for the previous API naming convention. The
scripts in XCB's tools directory will convert code written for the old API to
use the new API; they work well enough that we used them to convert the
non-program-generated code in XCB, and when run on the old program-generated
code, they almost exactly reproduce the new program-generated code (modulo
whitespace and bugs in the old code generator).
Authors: Vincent Torri, Thomas Hunger, Josh Triplett
|
|
|
|
|
|
This simplifies the patch for bug #8208 later.
|
|
|
|
|
|
|
|
|
|
Create a macro, XCB_SEQUENCE_COMPARE, that accepts two sequence numbers and
a comparison operator and correctly handles 32-bit wrap around.
Rewrite all ordered sequence number comparisons to use this macro.
Also, caught one error where a sequence was stored in a signed int variable.
Push out a GetInputFocus request when the sequence number does wrap at 32
bits so that applications cannot see sequence 0 (as that is an error
indicator).
|