If lots of requests are send without one causing a reply, xcb can get confused
about the current sequence number of a reply. Document the requirements of an
external socket owner to avoid this problem.
The return_socket callback for xcb_take_socket() originally was supposed to
return the last sequence number used, but the version committed to libxcb never
actually had this signature. This fixes the function's documentation not to
mention this non-existent return value.
Signed-off-by: Uli Schlachter <psychon@znc.in>
Signed-off-by: Julien Danjou <julien@danjou.info>
If the initial get_peer_sock_name(getpeername ...) succeeds, the
pointer to allocated memory is overwritten by the later call to
get_peer_sock_name(getsockname ...). Fix that up by freeing
the allocated memory before overwriting the pointer.
Signed-off-by: Nick Bowler <nbowler@draconx.ca>
Signed-off-by: Julien Danjou <julien@danjou.info>
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>
This simplifies the critical section of xcb_send_request and fixes a
couple of subtle bugs:
- It's possible for xcb_send_request to need to issue two sync requests
before it can issue the real request. Previously, we counted sequence
numbers as if both were issued, but only one went out on the wire.
- The test for whether to sync at 32-bit sequence number wrap has been
incorrect since we switched to 64-bit sequence numbers internally.
This change means that if the output queue was already full and the
current request is bigger than the output queue, XCB will do one more
write syscall than it did before. But syncs are rare and small requests
are the norm, so this shouldn't be a measurable difference.
Signed-off-by: Jamey Sharp <jamey@minilop.net>
By calling memmove instead of memcpy, and walking the buffer backward
from the end, *_unserialize is safe to use in-place.
Signed-off-by: Peter Harris <pharris@opentext.com>
In support of this, consolidate the two static error_connection
definitions into one so we don't try to free the static out-of-memory
error_connection.
Commit by Josh Triplett and Jamey Sharp.
Signed-off-by: Josh Triplett <josh@joshtriplett.org>
Signed-off-by: Jamey Sharp <jamey@minilop.net>
If a client calls close(2) on the connection's file descriptor and then
flushes writes, libxcb causes a hang in the client.
Any flush eventually calls _xcb_out_send() with has the following loop:
while(ret && *count)
ret = _xcb_conn_wait(c, &c->out.cond, vector, count);
_xcb_conn_wait(), if built with USE_POLL, gets the POLLNVAL error. It only
checks for POLLIN and POLLOUT though, ignoring the error. Return value is 1,
count is unmodified, leaving us with an endless loop and a client hang.
XTS testcase Xlib3/XConnectionNumber triggers this bug. It creates a display
connection, closes its file descriptor, tries to send a no-op, and then expects
an error.
http://cgit.freedesktop.org/xorg/test/xts/tree/xts5/Xlib3/XConnectionNumber.m
If poll returned POLLHUP or POLLERR, we might see the same result.
If poll returns any event we didn't ask for, this patch causes
_xcb_conn_shutdown() to be invoked and an error returned. This matches the
behaviour if select(2) is used instead of poll(2): select(2) returns -1 and
EBADF for an already closed file descriptor.
I believe this fix both is safe and will handle any similar error. POSIX says
that the only bits poll is permitted to set in revents are those bits that were
set in events, plus POLLHUP, POLLERR, and POLLNVAL. So if we see any flags we
didn't ask for then something has gone wrong.
Patch inspired by earlier proposals from Peter Hutterer and Aaron
Plattner--thanks!
Reported-by: Peter Hutterer <peter.hutterer@who-t.net>
Reported-by: Aaron Plattner <aplattner@nvidia.com>
Signed-off-by: Jamey Sharp <jamey@minilop.net>
Reviewed-by: Aaron Plattner <aplattner@nvidia.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Tested-by: Aaron Plattner <aplattner@nvidia.com>
Cc: Peter Hutterer <peter.hutterer@who-t.net>
Cc: Dan Nicholson <dbn.lists@gmail.com>
Signed-off-by: Peter Harris <pharris@opentext.com>
Fixes the X Test Suite's XCloseDisplay-6 test, which has this (admittedly
ridiculous) behavior:
1. Create a window w.
2. Open two display connections, dpy1, and dpy2.
3. Grab the server using dpy1.
4. Fork.
5 (child). XSetProperty on w using dpy2.
5 (parent). Verify that no event was recieved on dpy1.
6 (parent). XCloseDisplay(dpy1).
6 (child). Verify that an event was received on dpy2.
It was failing because at step 6 (child), the server had not actually ungrabbed
yet because the file descriptor for dpy1 was still open in the child process.
Shutting down the socket during XCloseDisplay matches the behavior of non-XCB
Xlib, which calls shutdown() from _X11TransSocketDisconnect.
Thanks to Julien Cristau for noticing this.
Signed-off-by: Aaron Plattner <aplattner at nvidia.com>
Reviewed-by: Julien Cristau <jcristau@debian.org>
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>
- API compatibility with valueparam
- request _aux() auxiliary functions
- _serialize() and _unserialize() auxiliary functions
- new data type that allows mixing of fixed and variable size members
protocol and host are allocated in _xcb_parse_display but ownership of
them is passed to the caller. They have to be freed in
xcb_connect_to_display_with_auth_info.
Signed-off-by: Pauli Nieminen <ext-pauli.nieminen@nokia.com>
Signed-off-by: Peter Harris <pharris@opentext.com>
xcb_parse_display claims that there is no side effects when failing.
That requires _xcb_parse_display to free the memory in failure case.
Signed-off-by: Pauli Nieminen <ext-pauli.nieminen@nokia.com>
Signed-off-by: Peter Harris <pharris@opentext.com>
Support scenarios where host is not set and protocol is. eg:
DISPLAY=tcp/:0
as well as the "inet" and "inet6" alias for "tcp" for compatability
with Xlib
Signed-off-by: Jeremy Huddleston <jeremyhu@apple.com>
Reviewed-by: Jamey Sharp <jamey@minilop.net>
This is a regression found by tinderbox in previous commit:
xcb_util.c: In function '_xcb_open':
xcb_util.c:213: error: 'fd' undeclared (first use in this function)
There could be no upper limit on the length of a path according
to POSIX, therefore these macros may not be defined at all on
some systems (such as GNU Hurd).
Signed-off-by: Arnaud Fontaine <arnau@debian.org>
Reviewed-by: Peter Harris <pharris@opentext.com>
Regression found by tinderbox in 89b3485dad
xcb_util.c:31:27: error: sys/syslimits.h: No such file or directory
xcb_util.c: In function '_xcb_open':
xcb_util.c:148: error: 'PATH_MAX' undeclared (first use in this function)
Signed-off-by: Jeremy Huddleston <jeremyhu@apple.com>
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>
_xcb_out_send needs _xcb_conn_wait to store back its progress so it can
be reinvoked to pick up where it left off---but then _xcb_out_send
guarantees that it leaves either an empty output vector or a shut-down
connection, so *its* callers never care how much progress was made.
Signed-off-by: Jamey Sharp <jamey@minilop.net>
Reviewed-by: Josh Triplett <josh@freedesktop.org>
Depending on the process file limit, a file descriptor can be larger
than the capacity of fd_set. There is no portable way to create a
large enough fd_set at run-time. So we just fail if the file descriptor
number is too high and poll() is not available.
Reviewed-by: Jamey Sharp <jamey@minilop.net>
Signed-off-by: Julien Danjou <julien@danjou.info>
libxcb's 010e5661 (Fix XDM-AUTHORIZATION-1 (bug #14202)) mistakenly
inverted a few lines of code, making local socket authentication fail on
hpux and Hurd: when getpeername fails, sockname needs to be initialized
by getsockname before its address family can be checked.
Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
Signed-off-by: Julien Danjou <julien@danjou.info>
This saves the X11 connection from leaking into children processes.
On Linux, this is fully thread-safe using SOCK_CLOEXEC. On other
systems, there is a small race condition.
Signed-off-by: Julien Danjou <julien@danjou.info>
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>
This matches xtrans behaviour in SocketINETConnect, and makes it so apps
don't hang forever if their display dies.
Signed-off-by: Adam Jackson <ajax@redhat.com>
-no-undefined is needed to tell libtool a shared library can be built
on platforms which require all references to be statisfied at link time.
Signed-off-by: Jon TURNEY <jon.turney@dronecode.org.uk>
Signed-off-by: Julien Danjou <julien@danjou.info>
Also replace excessively clever use of bitwise OR with equivalent
addition.
Reported-by: Geoffrey Li <geoffrey@seitopos.com>
Signed-off-by: Jamey Sharp <jamey@minilop.net>
Signed-off-by: Josh Triplett <josh@joshtriplett.org>
With this patch, we know use correctly the socket address or peer
address for authentication purpose.
Signed-off-by: Julien Danjou <julien@danjou.info>
Many code was duplicated between xcb_connect_to_display_with_auth_info
and xcb_connect(). We merge both, since the difference is just about the
xcb_auth_info_t pointer being supplied, or not.
Signed-off-by: Julien Danjou <julien@danjou.info>
Local socket connections currently do not work on hurd-i386 because
xcb_auth calls getpeername() on the client socket, but hurd-i386 does
not implement anything in that case (I actually wonder what reasonable
value could be returned). In such case the xcb code does not actually
need the peer name anyway.
Signed-off-by: Julien Danjou <julien@danjou.info>
This fixes a bug where c_client.py wasn't generating *_end functions,
but expected them to exist in order to find the subsequent list's start.
Signed-off-by: Peter Harris <peter.harris@hummingbird.com>
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.
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.
The GNU/kFreeBSD (and BSDs in general) have a different
layout of struct sockaddr, sockaddr_in, sockaddr_un ...
The first member do not have to be "sa_family",
they also have "sa_len" field.
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.
Since several extensions named their errors like "BadFoo", this patch
results in names like XCB_EXT_BAD_BAD_FOO, which is really awful. Those
extensions are already kind of awful, as they produce structure names
like xcb_ext_bad_foo_error_t, which is redundant.
A patch that removes "Bad" from the XML extension descriptions, while
maintaining API and ABI compatibility in XCB, is needed before this
patch can be released.
This reverts commit 158c9b6ba1.
With this patch, `ico -threads 2` runs without deadlock.
Many thanks to Christoph Pfister <christophpfister@gmail.com> for
pointing out the problem, providing detailed analyses, explaining it to
me repeatedly until I understood what was going on, and proposing and
reviewing possible solutions.
Signed-off-by: Jamey Sharp <jamey@minilop.net>
Acked-by: Christoph Pfister <christophpfister@gmail.com>
But do still print a full backtrace, on platforms where that's
supported.
This commit follows the spirit of Novell's libxcb-sloppy-lock.diff.
I strongly opposed proposals like this one for a long time. Originally I
had a very good reason: libX11, when compiled to use XCB, would crash
soon after a locking correctness violation, so it was better to have an
informative assert failure than a mystifying crash soon after.
It took some time for me to realize that I'd changed the libX11
implementation (for unrelated reasons) so that it could survive most
invalid locking situations, as long as it wasn't actually being used
from multiple threads concurrently.
The other thing that has changed is that most of the code with incorrect
locking has now been fixed. The value of the assert is accordingly
lower.
However, remaining broken callers do need to be fixed. That's why libXCB
will still noisily print a stacktrace (if possible) on each assertion
failure, even when assert isn't actually invoked to abort() the program;
and that's why aborting is still default. This environment variable is
provided only for use as a temporary workaround for broken applications.
Signed-off-by: Jamey Sharp <jamey@minilop.net>
Acked-by: Josh Triplett <josh@freedesktop.org>
As you know there are some nasty libs / apps doing locking
incorrectly. In order to improve the information given to the user
when he encounters such a situation (people don't run apps in gdb
normally) I created the patch attached.
It's very non-intrusive (and affects only xlib/xcb, Josh told me on
irc that it could be useful for other areas too, personally I don't
think that it's really needed at other places ...).
Some same outputs and the discussion of them:
lxuser@pdln:/tmp$ ./main
Got a backtrace:
#0 /tmp/usr/lib/libxcb-xlib.so.0 [0xb7f9d728]
#1 /tmp/usr/lib/libxcb-xlib.so.0(xcb_xlib_unlock+0x31) [0xb7f9d861]
#2 ./test.so(function_a+0x11) [0xb7f9f3fd]
#3 ./test.so(function_b+0x11) [0xb7f9f410]
#4 ./main [0x80484a7]
#5 /lib/libc.so.6(__libc_start_main+0xdc) [0xb7e60ebc]
#6 ./main [0x80483f1]
main: xcb_xlib.c:82: xcb_xlib_unlock: Assertion `c->xlib.lock' failed.
Aborted
That's kinda the normal situation.
lxuser@pdln:/tmp$ ./main
Got a backtrace:
#0 /tmp/usr/lib/libxcb-xlib.so.0 [0xb7f90728]
#1 /tmp/usr/lib/libxcb-xlib.so.0(xcb_xlib_unlock+0x31) [0xb7f90861]
#2 /tmp/test.so [0xb7f923cd]
#3 /tmp/test.so(function_b+0x11) [0xb7f923e0]
#4 ./main [0x80484ab]
#5 /lib/libc.so.6(__libc_start_main+0xdc) [0xb7e53ebc]
#6 ./main [0x80483f1]
main: xcb_xlib.c:82: xcb_xlib_unlock: Assertion `c->xlib.lock' failed.
Aborted
There are two possible reasons that the name doesn't appear in #2:
a) a hidden symbol or a symbol with statical linkage in a library
b) a symbol in an app not compiled with -rdynamic.
But in both cases you still know _where_ the caller is.
Note that in this example test.so was compiled with
-fomit-frame-pointer; this isn't an issue as _one_ (= the caller)
stack trace is still valid (as long as you don't have the insane idea
to compile xcb with -fo-f-p).
Another issue that may appear is "tail call elimination" (some entries
are mysteriously missing; this is quite ugly, but you still get enough
information so that you can do something useful with the issue e.g. by
disassembling the relevant parts with gdb).
Signed-off-by: Jamey Sharp <jamey@minilop.net>
Avoid race condition when symlinking XML files.
When declaring a rule with many files as target, the rule is called
when any of them is requested, resulting in multiple for loops happening
during a make process using more than one job.
Also, use '$(LN_S) -f' rather than removing and recreating a file,
that one should be as supported as 'rm -f' and requires one less command.
configure supports using custom CFLAGS, so remove the --with-opt and
--with-debug options from configure.ac, and the corresponding usage of
COPTFLAGS and CDEBUGFLAGS in src/Makefile.am.
Change xcb_connect to pass the display number to _xcb_get_auth_info, which
passes it to get_authptr. This allows get_authptr to stop hacking the display
number out of the sockaddrs of various address families, such as
port - X_TCP_PORT, or the number after the last X in the UNIX socket path. This
also removes a portability bug introduced during the IPv6 changes: relying on
'\0'-termination of the UNIX socket path in a sockaddr_un.
Commit by Jamey Sharp and Josh Triplett.
xcb_parse_display already correctly handled IPv6 displays. Now, _xcb_open_tcp
uses getaddrinfo, and correctly connects to IPv6 displays. Displays can use
bare IPv6 addresses, square-bracketed IPv6 addresses, or hostnames which
resolve to IPv6 addresses.
Since IPv6 addresses may include colons, including at the end, revise the
DECnet display parsing code to avoid triggering on IPv6 addresses.
Authorization may not work with IPv6 connections yet.
This commit brought to you by the (display) number ::1:1.1, the letter X,
the Gobby collaborative editor, Josh Triplett, and Jamey Sharp.
Only one use of <localfield> remained, for a list length expression in
xv.xml. List length parameters that don't actually appear in the
protocol should be left implicit: if no length expression is given, then
a localfield will be automatically created by c-client.xsl.
Specifically, fixes these two warnings which were emitted for every
generated source file:
* Warning: end of file while inside a group
* Warning: group XCB_BigRequests_API already documented. Skipping documentation.
Hard coding the opcode numbers in the function just makes it harder to figure
out what's going on, but much more to the point, not defining the opcodes in
the header makes it impossible to use the generated headers instead of the
x11proto headers in the server.
The name I settled on is very simple, for an extension by the name of xconf,
and a request by the name of list_devices, we get XCB_XCONF_LIST_DEVICES. If
this somehow causes problems, we can probably add a _OP somewhere in there,
but.
Acked-by: Jamey Sharp <jamey@minilop.net>
Closes: #8641
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.
Since extensions no longer provide type-specific XID-generation functions,
xcb_generate_id now forms part of the xcb client API, rather than the
extension API; move it from xcbext.h to xcb.h accordingly.
After positive feedback from several people, we have decided to remove the XID
wrapper structures that attempted to provide C type safety, and replace them
with uint32_t typedefs. Feedback has indicated that these type-safety hacks
generated more trouble than help.
We will bump the libxcb soname at the next release.
We don't want to have to change the libxcb soname if we later manage to remove
the Xlib compatibility functions, and nothing except Xlib should ever use
them, so split them into a separate library.
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
The installed headers can't be relying on the presence of the internal
config.h defines, and it was breaking the xcb build for me as well due to
config.h not being included early enough.
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).
libXCBextname. To use extension extname, include extname.h and link with
-lXCBextname. This allows extensions to change without bumping the main
libXCB version.
bigreq and xc_misc remain in libXCB, because XCB uses them internally to make
big requests and to allocate XIDs, respectively.
Uli Schlachter
Nick Bowler
Jamey Sharp
Peter Harris
Josh Triplett
Aaron Plattner
Christoph Reimann
Pauli Nieminen
Jeremy Huddleston
Marcin Kościelnicki
Arnaud Fontaine
Jeetu Golani
Rémi Denis-Courmont
Samuel Thibault
Rémi Denis-Courmont
Julien Danjou
Adam Jackson
Eric Anholt
Yaakov Selkowitz
Julien Cristau
elupus
Bart Massey
Michael Ost
Jeremy Huddleston
Peter Harris
Josh Triplett
Henning Sten
Carsten Meier
Vincent Torri
Petr Salinger
Peter Hutterer
Oswald Buddenhagen
Eamon Walsh
Jeremy Kolb
Egbert Eich
Christoph Pfister
Ian Osgood
TORRI Vincent
TORRI Vincent
Diego 'Flameeyes' Pettenò
Jeremy Kolb jkolb@brandeis.edu
Zephaniah E. Hull
Jeremy Kolb
Alan Coopersmith
Bart Massey
Eric Anholt
Keith Packard
Donnie Berkholz
Patrick Caulfield