Entirely too long ago, we wrote: > Libraries like Xlib/XCB, 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. XCB > currently has an Xlib-specific lock, and allows Xlib to block XCB from > making requests. The attached patches to XCB replace 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. The attached patches represent version 3 of our handoff implementation. Since the version 2 patchset, we've made two changes to the handoff API and fixed a few bugs, and after a short period for review we intend to finally push these patches. - We removed the "read" parameter from xcb_take_socket, because we found that Xlib can't use it and we don't think that anything else can correctly use it either. We're also not sure we can provide any useful value for it anyway. - The return_socket callback no longer returns a sequence number; instead, xcb_writev takes an argument giving the number of requests in the buffer, and XCB can count how many requests have passed. Thanks to a great debugging session with keithp yesterday, we believe these patches are ready for release, pending review and some more testing. Of course there's always more work we'd like to do. We mailed a patch to the XCB list a few weeks ago adding a new request flag to drop any reply or error for that request into the event queue. For that to be useful, we've realized that the event queue needs to annotate its responses with 64-bit widened sequence numbers, and should do that with an out-parameter, not the "full_sequence" field inserted into the event itself. A lot more of our code in Xlib can go away if we make these small extensions to XCB's API, and the code that we could remove was hard to write and has been repeatedly buggy. - Josh Triplett and Jamey Sharp
From ae210575178ea559a8811911d5a7231118cd03a1 Mon Sep 17 00:00:00 2001
From: Jamey Sharp <jamey@minilop.net>
Date: Fri, 14 Mar 2008 12:08:32 -0700
Subject: [PATCH] Remove libxcb-xlib and xcbxlib.h.
---
Makefile.am | 4 +--
configure.ac | 1 -
src/Makefile.am | 9 +----
src/xcb_conn.c | 49 +------------------------------
src/xcb_xlib.c | 87 -------------------------------------------------------
src/xcbint.h | 19 +-----------
src/xcbxlib.h | 44 ----------------------------
xcb-xlib.pc.in | 11 -------
8 files changed, 6 insertions(+), 218 deletions(-)
delete mode 100644 src/xcb_xlib.c
delete mode 100644 src/xcbxlib.h
delete mode 100644 xcb-xlib.pc.in
diff --git a/Makefile.am b/Makefile.am
index ee97180..3a2f6f5 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -2,9 +2,7 @@ SUBDIRS=src tests doc
pkgconfigdir = $(libdir)/pkgconfig
-pkgconfig_DATA = \
-xcb.pc \
-xcb-xlib.pc
+pkgconfig_DATA = xcb.pc
if BUILD_COMPOSITE
pkgconfig_DATA += xcb-composite.pc
diff --git a/configure.ac b/configure.ac
index c3d3595..e9d9412 100644
--- a/configure.ac
+++ b/configure.ac
@@ -142,7 +142,6 @@ tests/Makefile
AC_CONFIG_FILES([
xcb.pc
-xcb-xlib.pc
xcb-composite.pc
xcb-damage.pc
xcb-dpms.pc
diff --git a/src/Makefile.am b/src/Makefile.am
index aa8b755..bf6e991 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -1,5 +1,4 @@
-lib_LTLIBRARIES = libxcb.la \
- libxcb-xlib.la
+lib_LTLIBRARIES = libxcb.la
EXTHEADERS = xproto.h \
bigreq.h \
@@ -29,10 +28,6 @@ libxcb_la_LDFLAGS = -version-info 1:0:0
XCB_LIBS = libxcb.la
-libxcb_xlib_la_LDFLAGS = -version-info 0:0:0
-libxcb_xlib_la_LIBADD = $(XCB_LIBS)
-libxcb_xlib_la_SOURCES = xcb_xlib.c
-
# FIXME: find a way to autogenerate this from the XML files.
EXTHEADERS += composite.h
@@ -257,7 +252,7 @@ endif
-xcbinclude_HEADERS = xcb.h xcbext.h xcbxlib.h $(EXTHEADERS)
+xcbinclude_HEADERS = xcb.h xcbext.h $(EXTHEADERS)
noinst_HEADERS = xcbint.h
BUILT_SOURCES = $(EXTSOURCES) $(EXTHEADERS)
diff --git a/src/xcb_conn.c b/src/xcb_conn.c
index 02f60bd..2bb8661 100644
--- a/src/xcb_conn.c
+++ b/src/xcb_conn.c
@@ -59,21 +59,6 @@ static int set_fd_flags(const int fd)
return 1;
}
-static int _xcb_xlib_init(_xcb_xlib *xlib)
-{
- xlib->lock = 0;
-#ifndef NDEBUG
- xlib->sloppy_lock = (getenv("LIBXCB_ALLOW_SLOPPY_LOCK") != 0);
-#endif
- pthread_cond_init(&xlib->cond, 0);
- return 1;
-}
-
-static void _xcb_xlib_destroy(_xcb_xlib *xlib)
-{
- pthread_cond_destroy(&xlib->cond);
-}
-
static int write_setup(xcb_connection_t *c, xcb_auth_info_t *auth_info)
{
static const char pad[3];
@@ -230,7 +215,6 @@ xcb_connection_t *xcb_connect_to_fd(int fd, xcb_auth_info_t *auth_info)
if(!(
set_fd_flags(fd) &&
pthread_mutex_init(&c->iolock, 0) == 0 &&
- _xcb_xlib_init(&c->xlib) &&
_xcb_in_init(&c->in) &&
_xcb_out_init(&c->out) &&
write_setup(c, auth_info) &&
@@ -255,7 +239,6 @@ void xcb_disconnect(xcb_connection_t *c)
close(c->fd);
pthread_mutex_destroy(&c->iolock);
- _xcb_xlib_destroy(&c->xlib);
_xcb_in_destroy(&c->in);
_xcb_out_destroy(&c->out);
@@ -275,12 +258,6 @@ void _xcb_conn_shutdown(xcb_connection_t *c)
void _xcb_lock_io(xcb_connection_t *c)
{
pthread_mutex_lock(&c->iolock);
- while(c->xlib.lock)
- {
- if(pthread_equal(c->xlib.thread, pthread_self()))
- break;
- pthread_cond_wait(&c->xlib.cond, &c->iolock);
- }
}
void _xcb_unlock_io(xcb_connection_t *c)
@@ -290,25 +267,12 @@ void _xcb_unlock_io(xcb_connection_t *c)
void _xcb_wait_io(xcb_connection_t *c, pthread_cond_t *cond)
{
- int xlib_locked = c->xlib.lock;
- if(xlib_locked)
- {
- c->xlib.lock = 0;
- pthread_cond_broadcast(&c->xlib.cond);
- }
pthread_cond_wait(cond, &c->iolock);
- if(xlib_locked)
- {
- while(c->xlib.lock)
- pthread_cond_wait(&c->xlib.cond, &c->iolock);
- c->xlib.lock = 1;
- c->xlib.thread = pthread_self();
- }
}
int _xcb_conn_wait(xcb_connection_t *c, pthread_cond_t *cond, struct iovec **vector, int *count)
{
- int ret, xlib_locked;
+ int ret;
fd_set rfds, wfds;
/* If the thing I should be doing is already being done, wait for it. */
@@ -329,12 +293,6 @@ int _xcb_conn_wait(xcb_connection_t *c, pthread_cond_t *cond, struct iovec **vec
++c->out.writing;
}
- xlib_locked = c->xlib.lock;
- if(xlib_locked)
- {
- c->xlib.lock = 0;
- pthread_cond_broadcast(&c->xlib.cond);
- }
_xcb_unlock_io(c);
do {
ret = select(c->fd + 1, &rfds, &wfds, 0, 0);
@@ -345,11 +303,6 @@ int _xcb_conn_wait(xcb_connection_t *c, pthread_cond_t *cond, struct iovec **vec
ret = 0;
}
_xcb_lock_io(c);
- if(xlib_locked)
- {
- c->xlib.lock = 1;
- c->xlib.thread = pthread_self();
- }
if(ret)
{
diff --git a/src/xcb_xlib.c b/src/xcb_xlib.c
deleted file mode 100644
index 1b573e8..0000000
--- a/src/xcb_xlib.c
+++ /dev/null
@@ -1,87 +0,0 @@
-/* Copyright (C) 2005 Bart Massey and Jamey Sharp.
- *
- * Permission is hereby granted, free of charge, to any person obtaining a
- * copy of this software and associated documentation files (the "Software"),
- * to deal in the Software without restriction, including without limitation
- * the rights to use, copy, modify, merge, publish, distribute, sublicense,
- * and/or sell copies of the Software, and to permit persons to whom the
- * Software is furnished to do so, subject to the following conditions:
- *
- * The above copyright notice and this permission notice shall be included in
- * all copies or substantial portions of the Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
- * AUTHORS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
- * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
- * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
- *
- * Except as contained in this notice, the names of the authors or their
- * institutions shall not be used in advertising or otherwise to promote the
- * sale, use or other dealings in this Software without prior written
- * authorization from the authors.
- */
-
-#include "xcbxlib.h"
-#include "xcbint.h"
-
-#include <assert.h>
-
-#ifdef HAVE_BACKTRACE
-#include <execinfo.h>
-#include <stdio.h>
-#include <stdlib.h>
-#endif
-
-static void xcb_xlib_printbt(void)
-{
-#ifdef HAVE_BACKTRACE
- void *array[20];
- int size;
- char **strings;
- int i;
-
- size = backtrace(array, 20);
- strings = backtrace_symbols(array, size);
-
- fprintf(stderr, "Locking assertion failure. Backtrace:\n");
-
- for (i = 0; i < size; ++i)
- fprintf(stderr, "#%i %s\n", i, strings[i]);
-
- free(strings);
-#endif
-}
-
-#ifndef NDEBUG
-#define xcb_assert(c,x) do { if (!(x)) { xcb_xlib_printbt(); if (!(c)->xlib.sloppy_lock) assert(x); } } while(0)
-#else
-#define xcb_assert(c,x)
-#endif
-
-unsigned int xcb_get_request_sent(xcb_connection_t *c)
-{
- if(c->has_error)
- return 0;
- return c->out.request;
-}
-
-void xcb_xlib_lock(xcb_connection_t *c)
-{
- _xcb_lock_io(c);
- xcb_assert(c, !c->xlib.lock);
- c->xlib.lock = 1;
- c->xlib.thread = pthread_self();
- _xcb_unlock_io(c);
-}
-
-void xcb_xlib_unlock(xcb_connection_t *c)
-{
- _xcb_lock_io(c);
- xcb_assert(c, c->xlib.lock);
- xcb_assert(c, pthread_equal(c->xlib.thread, pthread_self()));
- c->xlib.lock = 0;
- pthread_cond_broadcast(&c->xlib.cond);
- _xcb_unlock_io(c);
-}
diff --git a/src/xcbint.h b/src/xcbint.h
index ab0264f..86e00a4 100644
--- a/src/xcbint.h
+++ b/src/xcbint.h
@@ -126,16 +126,6 @@ int _xcb_in_read(xcb_connection_t *c);
int _xcb_in_read_block(xcb_connection_t *c, void *buf, int nread);
-/* xcb_xlib.c */
-
-typedef struct _xcb_xlib {
- int lock;
- int sloppy_lock;
- pthread_t thread;
- pthread_cond_t cond;
-} _xcb_xlib;
-
-
/* xcb_xid.c */
typedef struct _xcb_xid {
@@ -173,7 +163,6 @@ struct xcb_connection_t {
/* I/O data */
pthread_mutex_t iolock;
- _xcb_xlib xlib;
_xcb_in in;
_xcb_out out;
@@ -183,6 +172,8 @@ struct xcb_connection_t {
};
void _xcb_conn_shutdown(xcb_connection_t *c);
+void _xcb_lock_io(xcb_connection_t *c);
+void _xcb_unlock_io(xcb_connection_t *c);
void _xcb_wait_io(xcb_connection_t *c, pthread_cond_t *cond);
int _xcb_conn_wait(xcb_connection_t *c, pthread_cond_t *cond, struct iovec **vector, int *count);
@@ -195,10 +186,4 @@ int _xcb_get_auth_info(int fd, xcb_auth_info_t *info, int display);
#pragma GCC visibility pop
#endif
-
-/* xcb_conn.c symbols visible to xcb-xlib */
-
-void _xcb_lock_io(xcb_connection_t *c);
-void _xcb_unlock_io(xcb_connection_t *c);
-
#endif
diff --git a/src/xcbxlib.h b/src/xcbxlib.h
deleted file mode 100644
index 4cb5cd4..0000000
--- a/src/xcbxlib.h
+++ /dev/null
@@ -1,44 +0,0 @@
-/*
- * Copyright (C) 2005 Bart Massey and Jamey Sharp.
- * All Rights Reserved.
- *
- * Permission is hereby granted, free of charge, to any person obtaining a
- * copy of this software and associated documentation files (the "Software"),
- * to deal in the Software without restriction, including without limitation
- * the rights to use, copy, modify, merge, publish, distribute, sublicense,
- * and/or sell copies of the Software, and to permit persons to whom the
- * Software is furnished to do so, subject to the following conditions:
- *
- * The above copyright notice and this permission notice shall be included in
- * all copies or substantial portions of the Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
- * AUTHORS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
- * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
- * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
- *
- * Except as contained in this notice, the names of the authors or their
- * institutions shall not be used in advertising or otherwise to promote the
- * sale, use or other dealings in this Software without prior written
- * authorization from the authors.
- */
-
-/* This include file declares functions used by Xlib/XCB, but nothing else
- * should ever use these functions or link to libxcb-xlib. */
-
-#ifndef __XCBXLIB_H
-#define __XCBXLIB_H
-
-#include <pthread.h>
-#include "xcb.h"
-
-/* The caller of this function must hold the xlib lock, using the lock
- * functions below. */
-unsigned int xcb_get_request_sent(xcb_connection_t *c);
-
-void xcb_xlib_lock(xcb_connection_t *c);
-void xcb_xlib_unlock(xcb_connection_t *c);
-
-#endif
diff --git a/xcb-xlib.pc.in b/xcb-xlib.pc.in
deleted file mode 100644
index f28b529..0000000
--- a/xcb-xlib.pc.in
+++ /dev/null
@@ -1,11 +0,0 @@
-prefix=@prefix@
-exec_prefix=@exec_prefix@
-libdir=@libdir@
-includedir=@includedir@
-
-Name: XCB Xlib
-Description: XCB Xlib support functions
-Version: @PACKAGE_VERSION@
-Requires: xcb
-Libs: -L${libdir} -lxcb-xlib
-Cflags: -I${includedir}
--
1.5.6.5
From 027b59fbbe3d38e7fdd0119d99ae6aaee1dae065 Mon Sep 17 00:00:00 2001
From: Jamey Sharp <jamey@minilop.net>
Date: Fri, 14 Mar 2008 12:08:58 -0700
Subject: [PATCH] Inline _xcb_lock_io, _xcb_unlock_io, and _xcb_wait_io.
These functions are once again a single pthread call, so just make that
call directly.
---
src/xcb_conn.c | 25 +++++--------------------
src/xcb_in.c | 16 ++++++++--------
src/xcb_out.c | 12 ++++++------
src/xcbint.h | 3 ---
4 files changed, 19 insertions(+), 37 deletions(-)
diff --git a/src/xcb_conn.c b/src/xcb_conn.c
index 2bb8661..5b097f7 100644
--- a/src/xcb_conn.c
+++ b/src/xcb_conn.c
@@ -97,12 +97,12 @@ static int write_setup(xcb_connection_t *c, xcb_auth_info_t *auth_info)
}
assert(count <= (int) (sizeof(parts) / sizeof(*parts)));
- _xcb_lock_io(c);
+ pthread_mutex_lock(&c->iolock);
{
struct iovec *parts_ptr = parts;
ret = _xcb_out_send(c, &parts_ptr, &count);
}
- _xcb_unlock_io(c);
+ pthread_mutex_unlock(&c->iolock);
return ret;
}
@@ -255,21 +255,6 @@ void _xcb_conn_shutdown(xcb_connection_t *c)
c->has_error = 1;
}
-void _xcb_lock_io(xcb_connection_t *c)
-{
- pthread_mutex_lock(&c->iolock);
-}
-
-void _xcb_unlock_io(xcb_connection_t *c)
-{
- pthread_mutex_unlock(&c->iolock);
-}
-
-void _xcb_wait_io(xcb_connection_t *c, pthread_cond_t *cond)
-{
- pthread_cond_wait(cond, &c->iolock);
-}
-
int _xcb_conn_wait(xcb_connection_t *c, pthread_cond_t *cond, struct iovec **vector, int *count)
{
int ret;
@@ -278,7 +263,7 @@ int _xcb_conn_wait(xcb_connection_t *c, pthread_cond_t *cond, struct iovec **vec
/* If the thing I should be doing is already being done, wait for it. */
if(count ? c->out.writing : c->in.reading)
{
- _xcb_wait_io(c, cond);
+ pthread_cond_wait(cond, &c->iolock);
return 1;
}
@@ -293,7 +278,7 @@ int _xcb_conn_wait(xcb_connection_t *c, pthread_cond_t *cond, struct iovec **vec
++c->out.writing;
}
- _xcb_unlock_io(c);
+ pthread_mutex_unlock(&c->iolock);
do {
ret = select(c->fd + 1, &rfds, &wfds, 0, 0);
} while (ret == -1 && errno == EINTR);
@@ -302,7 +287,7 @@ int _xcb_conn_wait(xcb_connection_t *c, pthread_cond_t *cond, struct iovec **vec
_xcb_conn_shutdown(c);
ret = 0;
}
- _xcb_lock_io(c);
+ pthread_mutex_lock(&c->iolock);
if(ret)
{
diff --git a/src/xcb_in.c b/src/xcb_in.c
index f613772..a0e5e10 100644
--- a/src/xcb_in.c
+++ b/src/xcb_in.c
@@ -341,7 +341,7 @@ void *xcb_wait_for_reply(xcb_connection_t *c, unsigned int request, xcb_generic_
if(c->has_error)
return 0;
- _xcb_lock_io(c);
+ pthread_mutex_lock(&c->iolock);
/* If this request has not been written yet, write it. */
if(_xcb_out_flush_to(c, request))
@@ -381,7 +381,7 @@ void *xcb_wait_for_reply(xcb_connection_t *c, unsigned int request, xcb_generic_
}
wake_up_next_reader(c);
- _xcb_unlock_io(c);
+ pthread_mutex_unlock(&c->iolock);
return ret;
}
@@ -396,9 +396,9 @@ int xcb_poll_for_reply(xcb_connection_t *c, unsigned int request, void **reply,
return 1; /* would not block */
}
assert(reply != 0);
- _xcb_lock_io(c);
+ pthread_mutex_lock(&c->iolock);
ret = poll_for_reply(c, request, reply, error);
- _xcb_unlock_io(c);
+ pthread_mutex_unlock(&c->iolock);
return ret;
}
@@ -407,14 +407,14 @@ xcb_generic_event_t *xcb_wait_for_event(xcb_connection_t *c)
xcb_generic_event_t *ret;
if(c->has_error)
return 0;
- _xcb_lock_io(c);
+ pthread_mutex_lock(&c->iolock);
/* get_event returns 0 on empty list. */
while(!(ret = get_event(c)))
if(!_xcb_conn_wait(c, &c->in.event_cond, 0, 0))
break;
wake_up_next_reader(c);
- _xcb_unlock_io(c);
+ pthread_mutex_unlock(&c->iolock);
return ret;
}
@@ -423,12 +423,12 @@ xcb_generic_event_t *xcb_poll_for_event(xcb_connection_t *c)
xcb_generic_event_t *ret = 0;
if(!c->has_error)
{
- _xcb_lock_io(c);
+ pthread_mutex_lock(&c->iolock);
/* FIXME: follow X meets Z architecture changes. */
ret = get_event(c);
if(!ret && _xcb_in_read(c)) /* _xcb_in_read shuts down the connection on error */
ret = get_event(c);
- _xcb_unlock_io(c);
+ pthread_mutex_unlock(&c->iolock);
}
return ret;
}
diff --git a/src/xcb_out.c b/src/xcb_out.c
index 000b121..ad4240a 100644
--- a/src/xcb_out.c
+++ b/src/xcb_out.c
@@ -187,10 +187,10 @@ unsigned int xcb_send_request(xcb_connection_t *c, int flags, struct iovec *vect
workaround = WORKAROUND_GLX_GET_FB_CONFIGS_BUG;
/* get a sequence number and arrange for delivery. */
- _xcb_lock_io(c);
+ pthread_mutex_lock(&c->iolock);
/* wait for other writing threads to get out of my way. */
while(c->out.writing)
- _xcb_wait_io(c, &c->out.cond);
+ pthread_cond_wait(&c->out.cond, &c->iolock);
request = ++c->out.request;
/* send GetInputFocus (sync_req) when 64k-2 requests have been sent without
@@ -231,7 +231,7 @@ unsigned int xcb_send_request(xcb_connection_t *c, int flags, struct iovec *vect
_xcb_conn_shutdown(c);
request = 0;
}
- _xcb_unlock_io(c);
+ pthread_mutex_unlock(&c->iolock);
return request;
}
@@ -240,9 +240,9 @@ int xcb_flush(xcb_connection_t *c)
int ret;
if(c->has_error)
return 0;
- _xcb_lock_io(c);
+ pthread_mutex_lock(&c->iolock);
ret = _xcb_out_flush_to(c, c->out.request);
- _xcb_unlock_io(c);
+ pthread_mutex_unlock(&c->iolock);
return ret;
}
@@ -297,7 +297,7 @@ int _xcb_out_flush_to(xcb_connection_t *c, unsigned int request)
return _xcb_out_send(c, &vec_ptr, &count);
}
while(c->out.writing)
- _xcb_wait_io(c, &c->out.cond);
+ pthread_cond_wait(&c->out.cond, &c->iolock);
assert(XCB_SEQUENCE_COMPARE(c->out.request_written, >=, request));
return 1;
}
diff --git a/src/xcbint.h b/src/xcbint.h
index 86e00a4..d610a10 100644
--- a/src/xcbint.h
+++ b/src/xcbint.h
@@ -172,9 +172,6 @@ struct xcb_connection_t {
};
void _xcb_conn_shutdown(xcb_connection_t *c);
-void _xcb_lock_io(xcb_connection_t *c);
-void _xcb_unlock_io(xcb_connection_t *c);
-void _xcb_wait_io(xcb_connection_t *c, pthread_cond_t *cond);
int _xcb_conn_wait(xcb_connection_t *c, pthread_cond_t *cond, struct iovec **vector, int *count);
--
1.5.6.5
From 3012334957cb84d30d1f7981b0b30692fe69cd08 Mon Sep 17 00:00:00 2001
From: Jamey Sharp <jamey@minilop.net>
Date: Fri, 14 Mar 2008 20:18:52 -0700
Subject: [PATCH] Use sequence number ranges in pending replies
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.
---
src/xcb_in.c | 11 +++++++----
1 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/src/xcb_in.c b/src/xcb_in.c
index a0e5e10..0c68208 100644
--- a/src/xcb_in.c
+++ b/src/xcb_in.c
@@ -52,7 +52,8 @@ struct reply_list {
};
typedef struct pending_reply {
- unsigned int request;
+ unsigned int first_request;
+ unsigned int last_request;
enum workarounds workaround;
int flags;
struct pending_reply *next;
@@ -112,7 +113,7 @@ static int read_packet(xcb_connection_t *c)
}
while(c->in.pending_replies &&
- XCB_SEQUENCE_COMPARE (c->in.pending_replies->request, <=, c->in.request_completed))
+ XCB_SEQUENCE_COMPARE (c->in.pending_replies->last_request, <=, c->in.request_completed))
{
pending_reply *oldpend = c->in.pending_replies;
c->in.pending_replies = oldpend->next;
@@ -128,7 +129,9 @@ static int read_packet(xcb_connection_t *c)
if(genrep.response_type == XCB_ERROR || genrep.response_type == XCB_REPLY)
{
pend = c->in.pending_replies;
- if(pend && pend->request != c->in.request_read)
+ if(pend &&
+ (XCB_SEQUENCE_COMPARE(c->in.request_read, <, pend->first_request) ||
+ XCB_SEQUENCE_COMPARE(c->in.request_read, >, pend->last_request)))
pend = 0;
}
@@ -506,7 +509,7 @@ int _xcb_in_expect_reply(xcb_connection_t *c, unsigned int request, enum workaro
_xcb_conn_shutdown(c);
return 0;
}
- pend->request = request;
+ pend->first_request = pend->last_request = request;
pend->workaround = workaround;
pend->flags = flags;
pend->next = 0;
--
1.5.6.5
From 0a403b5613a5c6a3c076fa2d50517c0a93208586 Mon Sep 17 00:00:00 2001
From: Jamey Sharp <jamey@minilop.net>
Date: Wed, 21 May 2008 14:44:16 -0700
Subject: [PATCH] Track 64-bit sequence numbers internally.
External APIs that used 32-bit sequence numbers continue to do so.
Commit by Josh Triplett and Jamey Sharp.
---
src/xcb_in.c | 35 ++++++++++++++++++++---------------
src/xcb_out.c | 4 ++--
src/xcbint.h | 17 +++++++++--------
3 files changed, 31 insertions(+), 25 deletions(-)
diff --git a/src/xcb_in.c b/src/xcb_in.c
index 0c68208..6bf6648 100644
--- a/src/xcb_in.c
+++ b/src/xcb_in.c
@@ -52,8 +52,8 @@ struct reply_list {
};
typedef struct pending_reply {
- unsigned int first_request;
- unsigned int last_request;
+ uint64_t first_request;
+ uint64_t last_request;
enum workarounds workaround;
int flags;
struct pending_reply *next;
@@ -94,8 +94,8 @@ static int read_packet(xcb_connection_t *c)
/* Compute 32-bit sequence number of this packet. */
if((genrep.response_type & 0x7f) != XCB_KEYMAP_NOTIFY)
{
- unsigned int lastread = c->in.request_read;
- c->in.request_read = (lastread & 0xffff0000) | genrep.sequence;
+ uint64_t lastread = c->in.request_read;
+ c->in.request_read = (lastread & UINT64_C(0xffffffffffff0000)) | genrep.sequence;
if(XCB_SEQUENCE_COMPARE(c->in.request_read, <, lastread))
c->in.request_read += 0x10000;
if(XCB_SEQUENCE_COMPARE(c->in.request_read, >, c->in.request_expected))
@@ -203,10 +203,10 @@ static int read_packet(xcb_connection_t *c)
c->in.current_reply_tail = &cur->next;
for(reader = c->in.readers;
reader &&
- XCB_SEQUENCE_COMPARE(reader->request, <=, c->in.request_read);
+ XCB_SEQUENCE_COMPARE_32(reader->request, <=, c->in.request_read);
reader = reader->next)
{
- if(reader->request == c->in.request_read)
+ if(XCB_SEQUENCE_COMPARE_32(reader->request, ==, c->in.request_read))
{
pthread_cond_signal(reader->data);
break;
@@ -288,7 +288,7 @@ static int poll_for_reply(xcb_connection_t *c, unsigned int request, void **repl
head = 0;
/* We've read requests past the one we want, so if it has replies we have
* them all and they're in the replies map. */
- else if(XCB_SEQUENCE_COMPARE(request, <, c->in.request_read))
+ else if(XCB_SEQUENCE_COMPARE_32(request, <, c->in.request_read))
{
head = _xcb_map_remove(c->in.replies, request);
if(head && head->next)
@@ -296,7 +296,7 @@ static int poll_for_reply(xcb_connection_t *c, unsigned int request, void **repl
}
/* We're currently processing the responses to the request we want, and we
* have a reply ready to return. So just return it without blocking. */
- else if(request == c->in.request_read && c->in.current_reply)
+ else if(XCB_SEQUENCE_COMPARE_32(request, ==, c->in.request_read) && c->in.current_reply)
{
head = c->in.current_reply;
c->in.current_reply = head->next;
@@ -305,7 +305,7 @@ static int poll_for_reply(xcb_connection_t *c, unsigned int request, void **repl
}
/* We know this request can't have any more replies, and we've already
* established it doesn't have a reply now. Don't bother blocking. */
- else if(request == c->in.request_completed)
+ else if(XCB_SEQUENCE_COMPARE_32(request, ==, c->in.request_completed))
head = 0;
/* We may have more replies on the way for this request: block until we're
* sure. */
@@ -338,6 +338,7 @@ static int poll_for_reply(xcb_connection_t *c, unsigned int request, void **repl
void *xcb_wait_for_reply(xcb_connection_t *c, unsigned int request, xcb_generic_error_t **e)
{
+ uint64_t widened_request;
void *ret = 0;
if(e)
*e = 0;
@@ -346,8 +347,12 @@ void *xcb_wait_for_reply(xcb_connection_t *c, unsigned int request, xcb_generic_
pthread_mutex_lock(&c->iolock);
+ widened_request = (c->out.request & UINT64_C(0xffffffff00000000)) | request;
+ if(widened_request > c->out.request)
+ widened_request -= UINT64_C(1) << 32;
+
/* If this request has not been written yet, write it. */
- if(_xcb_out_flush_to(c, request))
+ if(_xcb_out_flush_to(c, widened_request))
{
pthread_cond_t cond = PTHREAD_COND_INITIALIZER;
reader_list reader;
@@ -355,7 +360,7 @@ void *xcb_wait_for_reply(xcb_connection_t *c, unsigned int request, xcb_generic_
for(prev_reader = &c->in.readers;
*prev_reader &&
- XCB_SEQUENCE_COMPARE ((*prev_reader)->request, <=, request);
+ XCB_SEQUENCE_COMPARE_32((*prev_reader)->request, <=, request);
prev_reader = &(*prev_reader)->next)
{
/* empty */;
@@ -371,7 +376,7 @@ void *xcb_wait_for_reply(xcb_connection_t *c, unsigned int request, xcb_generic_
for(prev_reader = &c->in.readers;
*prev_reader &&
- XCB_SEQUENCE_COMPARE((*prev_reader)->request, <=, request);
+ XCB_SEQUENCE_COMPARE_32((*prev_reader)->request, <=, request);
prev_reader = &(*prev_reader)->next)
{
if(*prev_reader == &reader)
@@ -445,8 +450,8 @@ xcb_generic_error_t *xcb_request_check(xcb_connection_t *c, xcb_void_cookie_t co
void *reply;
if(c->has_error)
return 0;
- if(XCB_SEQUENCE_COMPARE(cookie.sequence,>,c->in.request_expected)
- && XCB_SEQUENCE_COMPARE(cookie.sequence,>,c->in.request_completed))
+ if(XCB_SEQUENCE_COMPARE_32(cookie.sequence,>,c->in.request_expected)
+ && XCB_SEQUENCE_COMPARE_32(cookie.sequence,>,c->in.request_completed))
{
free(xcb_get_input_focus_reply(c, xcb_get_input_focus(c), &ret));
assert(!ret);
@@ -500,7 +505,7 @@ void _xcb_in_destroy(_xcb_in *in)
}
}
-int _xcb_in_expect_reply(xcb_connection_t *c, unsigned int request, enum workarounds workaround, int flags)
+int _xcb_in_expect_reply(xcb_connection_t *c, uint64_t request, enum workarounds workaround, int flags)
{
pending_reply *pend = malloc(sizeof(pending_reply));
assert(workaround != WORKAROUND_NONE || flags != 0);
diff --git a/src/xcb_out.c b/src/xcb_out.c
index ad4240a..1094ceb 100644
--- a/src/xcb_out.c
+++ b/src/xcb_out.c
@@ -112,7 +112,7 @@ unsigned int xcb_send_request(xcb_connection_t *c, int flags, struct iovec *vect
} fields;
uint32_t packet;
} sync_req = { { /* GetInputFocus */ 43, 0, 1 } };
- unsigned int request;
+ uint64_t request;
uint32_t prefix[3] = { 0 };
int veclen = req->count;
enum workarounds workaround = WORKAROUND_NONE;
@@ -282,7 +282,7 @@ int _xcb_out_send(xcb_connection_t *c, struct iovec **vector, int *count)
return ret;
}
-int _xcb_out_flush_to(xcb_connection_t *c, unsigned int request)
+int _xcb_out_flush_to(xcb_connection_t *c, uint64_t request)
{
assert(XCB_SEQUENCE_COMPARE(request, <=, c->out.request));
if(XCB_SEQUENCE_COMPARE(c->out.request_written, >=, request))
diff --git a/src/xcbint.h b/src/xcbint.h
index d610a10..cd351de 100644
--- a/src/xcbint.h
+++ b/src/xcbint.h
@@ -52,7 +52,8 @@ enum lazy_reply_tag
#define XCB_PAD(i) (-(i) & 3)
-#define XCB_SEQUENCE_COMPARE(a,op,b) ((int) ((a) - (b)) op 0)
+#define XCB_SEQUENCE_COMPARE(a,op,b) ((int64_t) ((a) - (b)) op 0)
+#define XCB_SEQUENCE_COMPARE_32(a,op,b) (((int) (a) - (int) (b)) op 0)
/* xcb_list.c */
@@ -75,8 +76,8 @@ typedef struct _xcb_out {
char queue[4096];
int queue_len;
- unsigned int request;
- unsigned int request_written;
+ uint64_t request;
+ uint64_t request_written;
pthread_mutex_t reqlenlock;
enum lazy_reply_tag maximum_request_length_tag;
@@ -90,7 +91,7 @@ int _xcb_out_init(_xcb_out *out);
void _xcb_out_destroy(_xcb_out *out);
int _xcb_out_send(xcb_connection_t *c, struct iovec **vector, int *count);
-int _xcb_out_flush_to(xcb_connection_t *c, unsigned int request);
+int _xcb_out_flush_to(xcb_connection_t *c, uint64_t request);
/* xcb_in.c */
@@ -102,9 +103,9 @@ typedef struct _xcb_in {
char queue[4096];
int queue_len;
- unsigned int request_expected;
- unsigned int request_read;
- unsigned int request_completed;
+ uint64_t request_expected;
+ uint64_t request_read;
+ uint64_t request_completed;
struct reply_list *current_reply;
struct reply_list **current_reply_tail;
@@ -120,7 +121,7 @@ typedef struct _xcb_in {
int _xcb_in_init(_xcb_in *in);
void _xcb_in_destroy(_xcb_in *in);
-int _xcb_in_expect_reply(xcb_connection_t *c, unsigned int request, enum workarounds workaround, int flags);
+int _xcb_in_expect_reply(xcb_connection_t *c, uint64_t request, enum workarounds workaround, int flags);
int _xcb_in_read(xcb_connection_t *c);
int _xcb_in_read_block(xcb_connection_t *c, void *buf, int nread);
--
1.5.6.5
From 67438753bef0d52e5a4a74bd15c444196a809b2b Mon Sep 17 00:00:00 2001
From: Josh Triplett <josh@freedesktop.org>
Date: Sun, 16 Mar 2008 23:16:31 -0700
Subject: [PATCH] Support handing off socket write permission to external code.
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.
---
src/Makefile.am | 2 +-
src/xcb_in.c | 22 +++++++++++++++++--
src/xcb_out.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
src/xcbext.h | 17 +++++++++++++++
src/xcbint.h | 15 +++++++++++++-
5 files changed, 110 insertions(+), 5 deletions(-)
diff --git a/src/Makefile.am b/src/Makefile.am
index bf6e991..756337a 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -24,7 +24,7 @@ libxcb_la_SOURCES = \
# * If you add an interface, increment current and age and set revision to 0.
# * If you change or remove an interface, increment current and set revision
# and age to 0.
-libxcb_la_LDFLAGS = -version-info 1:0:0
+libxcb_la_LDFLAGS = -version-info 2:0:1
XCB_LIBS = libxcb.la
diff --git a/src/xcb_in.c b/src/xcb_in.c
index 6bf6648..212dc9a 100644
--- a/src/xcb_in.c
+++ b/src/xcb_in.c
@@ -113,6 +113,7 @@ static int read_packet(xcb_connection_t *c)
}
while(c->in.pending_replies &&
+ c->in.pending_replies->workaround != WORKAROUND_EXTERNAL_SOCKET_OWNER &&
XCB_SEQUENCE_COMPARE (c->in.pending_replies->last_request, <=, c->in.request_completed))
{
pending_reply *oldpend = c->in.pending_replies;
@@ -130,8 +131,9 @@ static int read_packet(xcb_connection_t *c)
{
pend = c->in.pending_replies;
if(pend &&
- (XCB_SEQUENCE_COMPARE(c->in.request_read, <, pend->first_request) ||
- XCB_SEQUENCE_COMPARE(c->in.request_read, >, pend->last_request)))
+ !(XCB_SEQUENCE_COMPARE(pend->first_request, <=, c->in.request_read) &&
+ (pend->workaround == WORKAROUND_EXTERNAL_SOCKET_OWNER ||
+ XCB_SEQUENCE_COMPARE(c->in.request_read, <=, pend->last_request))))
pend = 0;
}
@@ -352,7 +354,7 @@ void *xcb_wait_for_reply(xcb_connection_t *c, unsigned int request, xcb_generic_
widened_request -= UINT64_C(1) << 32;
/* If this request has not been written yet, write it. */
- if(_xcb_out_flush_to(c, widened_request))
+ if(c->out.return_socket || _xcb_out_flush_to(c, widened_request))
{
pthread_cond_t cond = PTHREAD_COND_INITIALIZER;
reader_list reader;
@@ -523,6 +525,20 @@ int _xcb_in_expect_reply(xcb_connection_t *c, uint64_t request, enum workarounds
return 1;
}
+void _xcb_in_replies_done(xcb_connection_t *c)
+{
+ struct pending_reply *pend;
+ if (c->in.pending_replies_tail != &c->in.pending_replies)
+ {
+ pend = container_of(c->in.pending_replies_tail, struct pending_reply, next);
+ if(pend->workaround == WORKAROUND_EXTERNAL_SOCKET_OWNER)
+ {
+ pend->last_request = c->out.request;
+ pend->workaround = WORKAROUND_NONE;
+ }
+ }
+}
+
int _xcb_in_read(xcb_connection_t *c)
{
int n = read(c->fd, c->in.queue + c->in.queue_len, sizeof(c->in.queue) - c->in.queue_len);
diff --git a/src/xcb_out.c b/src/xcb_out.c
index 1094ceb..4c6ab13 100644
--- a/src/xcb_out.c
+++ b/src/xcb_out.c
@@ -55,6 +55,25 @@ static int write_block(xcb_connection_t *c, struct iovec *vector, int count)
return _xcb_out_send(c, &vector, &count);
}
+static void get_socket_back(xcb_connection_t *c)
+{
+ while(c->out.return_socket && c->out.socket_moving)
+ pthread_cond_wait(&c->out.socket_cond, &c->iolock);
+ if(!c->out.return_socket)
+ return;
+
+ c->out.socket_moving = 1;
+ pthread_mutex_unlock(&c->iolock);
+ c->out.return_socket(c->out.socket_closure);
+ pthread_mutex_lock(&c->iolock);
+ c->out.socket_moving = 0;
+
+ pthread_cond_broadcast(&c->out.socket_cond);
+ c->out.return_socket = 0;
+ c->out.socket_closure = 0;
+ _xcb_in_replies_done(c);
+}
+
/* Public interface */
void xcb_prefetch_maximum_request_length(xcb_connection_t *c)
@@ -191,6 +210,7 @@ unsigned int xcb_send_request(xcb_connection_t *c, int flags, struct iovec *vect
/* wait for other writing threads to get out of my way. */
while(c->out.writing)
pthread_cond_wait(&c->out.cond, &c->iolock);
+ get_socket_back(c);
request = ++c->out.request;
/* send GetInputFocus (sync_req) when 64k-2 requests have been sent without
@@ -235,6 +255,39 @@ unsigned int xcb_send_request(xcb_connection_t *c, int flags, struct iovec *vect
return request;
}
+int xcb_take_socket(xcb_connection_t *c, void (*return_socket)(void *closure), void *closure, int flags, uint64_t *sent)
+{
+ int ret;
+ if(c->has_error)
+ return 0;
+ pthread_mutex_lock(&c->iolock);
+ get_socket_back(c);
+ ret = _xcb_out_flush_to(c, c->out.request);
+ if(ret)
+ {
+ c->out.return_socket = return_socket;
+ c->out.socket_closure = closure;
+ if(flags)
+ _xcb_in_expect_reply(c, c->out.request, WORKAROUND_EXTERNAL_SOCKET_OWNER, flags);
+ assert(c->out.request == c->out.request_written);
+ *sent = c->out.request;
+ }
+ pthread_mutex_unlock(&c->iolock);
+ return ret;
+}
+
+int xcb_writev(xcb_connection_t *c, struct iovec *vector, int count, uint64_t requests)
+{
+ int ret;
+ if(c->has_error)
+ return 0;
+ pthread_mutex_lock(&c->iolock);
+ c->out.request += requests;
+ ret = _xcb_out_send(c, &vector, &count);
+ pthread_mutex_unlock(&c->iolock);
+ return ret;
+}
+
int xcb_flush(xcb_connection_t *c)
{
int ret;
@@ -250,6 +303,12 @@ int xcb_flush(xcb_connection_t *c)
int _xcb_out_init(_xcb_out *out)
{
+ if(pthread_cond_init(&out->socket_cond, 0))
+ return 0;
+ out->return_socket = 0;
+ out->socket_closure = 0;
+ out->socket_moving = 0;
+
if(pthread_cond_init(&out->cond, 0))
return 0;
out->writing = 0;
diff --git a/src/xcbext.h b/src/xcbext.h
index 01dd590..2e10ba2 100644
--- a/src/xcbext.h
+++ b/src/xcbext.h
@@ -59,6 +59,23 @@ enum xcb_send_request_flags_t {
unsigned int xcb_send_request(xcb_connection_t *c, int flags, struct iovec *vector, const xcb_protocol_request_t *request);
+/* xcb_take_socket allows external code to ask XCB for permission to
+ * take over the write side of the socket and send raw data with
+ * xcb_writev. xcb_take_socket provides the sequence number of the last
+ * request XCB sent. 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. */
+int xcb_take_socket(xcb_connection_t *c, void (*return_socket)(void *closure), void *closure, int flags, uint64_t *sent);
+
+/* You must own the write-side of the socket (you've called
+ * xcb_take_socket, and haven't returned from return_socket yet) to call
+ * xcb_writev. Also, the iovec must have at least 1 byte of data in it.
+ * */
+int xcb_writev(xcb_connection_t *c, struct iovec *vector, int count, uint64_t requests);
+
/* xcb_in.c */
diff --git a/src/xcbint.h b/src/xcbint.h
index cd351de..2c9f7cd 100644
--- a/src/xcbint.h
+++ b/src/xcbint.h
@@ -40,7 +40,8 @@
enum workarounds {
WORKAROUND_NONE,
- WORKAROUND_GLX_GET_FB_CONFIGS_BUG
+ WORKAROUND_GLX_GET_FB_CONFIGS_BUG,
+ WORKAROUND_EXTERNAL_SOCKET_OWNER
};
enum lazy_reply_tag
@@ -55,6 +56,12 @@ enum lazy_reply_tag
#define XCB_SEQUENCE_COMPARE(a,op,b) ((int64_t) ((a) - (b)) op 0)
#define XCB_SEQUENCE_COMPARE_32(a,op,b) (((int) (a) - (int) (b)) op 0)
+#ifndef offsetof
+#define offsetof(type,member) ((size_t) &((type *)0)->member)
+#endif
+
+#define container_of(pointer,type,member) ((type *)(((char *)(pointer)) - offsetof(type, member)))
+
/* xcb_list.c */
typedef void (*xcb_list_free_func_t)(void *);
@@ -73,6 +80,11 @@ typedef struct _xcb_out {
pthread_cond_t cond;
int writing;
+ pthread_cond_t socket_cond;
+ void (*return_socket)(void *closure);
+ void *socket_closure;
+ int socket_moving;
+
char queue[4096];
int queue_len;
@@ -122,6 +134,7 @@ int _xcb_in_init(_xcb_in *in);
void _xcb_in_destroy(_xcb_in *in);
int _xcb_in_expect_reply(xcb_connection_t *c, uint64_t request, enum workarounds workaround, int flags);
+void _xcb_in_replies_done(xcb_connection_t *c);
int _xcb_in_read(xcb_connection_t *c);
int _xcb_in_read_block(xcb_connection_t *c, void *buf, int nread);
--
1.5.6.5
From 1819224ff28a9f2c5d5229a3a841e439a84cc34f Mon Sep 17 00:00:00 2001
From: Josh Triplett <josh@freedesktop.org>
Date: Sat, 15 Mar 2008 18:17:41 -0700
Subject: [PATCH] Bump version number to 1.2
---
configure.ac | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/configure.ac b/configure.ac
index e9d9412..8685608 100644
--- a/configure.ac
+++ b/configure.ac
@@ -3,7 +3,7 @@
AC_PREREQ(2.57)
AC_INIT([libxcb],
- 1.1.90.1,
+ 1.2,
[xcb@lists.freedesktop.org])
AC_CONFIG_SRCDIR([xcb.pc.in])
AM_INIT_AUTOMAKE([foreign dist-bzip2])
--
1.5.6.5
From 50d5c8363da0cd10217d4f24e8561ed9a1e1881e Mon Sep 17 00:00:00 2001
From: Josh Triplett <josh@freedesktop.org>
Date: Sat, 15 Mar 2008 17:22:23 -0700
Subject: [PATCH] Use XCB's new socket handoff mechanism rather than the old XCB Xlib lock.
Previously, Xlib/XCB used XCB's Xlib lock to prevent XCB from sending
requests between calls to Xlib's LockDisplay and UnlockDisplay macros.
Xlib/XCB then sent all of its requests using XCB's xcb_send_request, and
had to flush its requests when unlocking the display.
XCB 1.2 adds a new socket handoff mechanism, xcb_take_socket. Replace
much of the existing Xlib/XCB implementation with the use of
xcb_take_socket to take ownership of the write side of the X connection
socket, and a return_socket callback which writes any outstanding requests
with xcb_writev. This approach allows Xlib/XCB to use the same buffering
as traditional Xlib did. In particular, programs which use Xlib/XCB and
never make XCB calls will never need to hand the socket back to XCB, and
vice versa.
This allows us to discard large quantities of synchronization code from
Xlib/XCB, together with the synchronization bugs present in that code.
Several test cases which previously failed now work perfectly, including
multi-threaded ico. In addition, the infamous locking correctness
assertions, triggered when double-locking or when unlocking without a
previous lock, no longer exist, because Xlib/XCB no longer has any reason
to care more about application locking than traditional Xlib does.
Furthermore, the handoff approach provides great improvements to
performance. Results from x11perf's XNoOp test, which represented the
worst case for the lock-based Xlib/XCB:
Traditional Xlib: average 19100000/sec
Lock-based Xlib/XCB: average 3350000/sec
Handoff-based Xlib/XCB: average 17400000/sec
Thus, for no-ops, the handoff mechanism provides more than a 4x speedup to
Xlib/XCB, bringing Xlib/XCB within 9% of traditional Xlib no-op
performance. Of course, real-world workloads do not use no-op, so your
mileage may vary. In particular, since no-ops represent the worst case,
we expect real workloads to more closely match the performance of
traditional Xlib.
While removing synchronization code, we changed _XReply to not drop
any locks when calling xcb_wait_for_reply; previously, we had to carefully
avoid a deadlock between the Display lock and the XCB Xlib lock. Holding
the locks reduces implementation complexity and should not impact applications.
Commit by Jamey Sharp and Josh Triplett.
XCB's handoff mechanism inspired by Keith Packard.
---
configure.ac | 4 +-
src/Makefile.am | 1 -
src/OpenDis.c | 27 +++----
src/Xxcbint.h | 27 +-----
src/xcb_io.c | 194 +++++++++++++++++++++++++++++-----------------
src/xcb_lock.c | 235 -------------------------------------------------------
6 files changed, 140 insertions(+), 348 deletions(-)
delete mode 100644 src/xcb_lock.c
diff --git a/configure.ac b/configure.ac
index 5febfb3..6141fa4 100644
--- a/configure.ac
+++ b/configure.ac
@@ -52,8 +52,8 @@ no)
AC_DEFINE(USE_XCB, 0, [Use XCB for low-level protocol implementation])
;;
*)
- X11_REQUIRES="xcb-xlib >= 1.1.90"
- X11_EXTRA_DEPS="xcb-xlib"
+ X11_REQUIRES="xcb >= 1.2"
+ X11_EXTRA_DEPS="xcb >= 1.2"
xdmauth="no" # XCB handles all auth
AC_DEFINE(USE_XCB, 1, [Use XCB for low-level protocol implementation])
;;
diff --git a/src/Makefile.am b/src/Makefile.am
index 564e03f..3380f81 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -342,7 +342,6 @@ EXTRA_DIST = \
if XCB
libX11_la_SOURCES += \
- xcb_lock.c \
xcb_disp.c \
xcb_io.c \
Xxcbint.h
diff --git a/src/OpenDis.c b/src/OpenDis.c
index d5089e8..aa2541f 100644
--- a/src/OpenDis.c
+++ b/src/OpenDis.c
@@ -261,13 +261,6 @@ XOpenDisplay (
return(NULL);
}
-#if USE_XCB
- if (!_XCBInitDisplayLock(dpy)) {
- OutOfMemory (dpy, setup);
- return(NULL);
- }
-#endif
-
if (!_XPollfdCacheInit(dpy)) {
OutOfMemory (dpy, setup);
return(NULL);
@@ -292,6 +285,10 @@ XOpenDisplay (
return(NULL);
}
dpy->bufmax = dpy->buffer + conn_buf_size;
+#if USE_XCB
+ dpy->xcb->real_bufmax = dpy->bufmax;
+ dpy->bufmax = dpy->buffer;
+#endif
/* Set up the input event queue and input event queue parameters. */
dpy->head = dpy->tail = NULL;
@@ -652,6 +649,12 @@ XOpenDisplay (
UnlockDisplay(dpy);
#endif /* !USE_XCB */
+#if USE_XCB
+ dpy->bigreq_size = xcb_get_maximum_request_length(dpy->xcb->connection);
+ if(dpy->bigreq_size <= dpy->max_request_size)
+ dpy->bigreq_size = 0;
+#endif /* USE_XCB */
+
/*
* Set up other stuff clients are always going to use.
*/
@@ -673,12 +676,6 @@ XOpenDisplay (
*/
(void) XSynchronize(dpy, _Xdebug);
-#if USE_XCB
- dpy->bigreq_size = xcb_get_maximum_request_length(dpy->xcb->connection);
- if(dpy->bigreq_size <= dpy->max_request_size)
- dpy->bigreq_size = 0;
-#endif /* USE_XCB */
-
/*
* get availability of large requests, and
* get the resource manager database off the root window.
@@ -883,10 +880,6 @@ void _XFreeDisplayStructure(dpy)
Xfree (dpy->scratch_buffer);
FreeDisplayLock(dpy);
-#if USE_XCB
- _XCBShutdownDisplayLock(dpy);
-#endif /* USE_XCB */
-
if (dpy->qfree) {
register _XQEvent *qelt = dpy->qfree;
diff --git a/src/Xxcbint.h b/src/Xxcbint.h
index cc73749..1b3137b 100644
--- a/src/Xxcbint.h
+++ b/src/Xxcbint.h
@@ -5,34 +5,28 @@
#define XXCBINT_H
#include <assert.h>
+#include <stdint.h>
#include <X11/Xlibint.h>
#include <X11/Xlib-xcb.h>
-#include "locking.h" /* really just want X11/Xthreads.h but can't have it. */
-#define XCB_SEQUENCE_COMPARE(a,op,b) ((int) ((a) - (b)) op 0)
-#define assert_sequence_less(a,b) assert(XCB_SEQUENCE_COMPARE((a), <=, (b)))
+#define XLIB_SEQUENCE_COMPARE(a,op,b) (((long) (a) - (long) (b)) op 0)
typedef struct PendingRequest PendingRequest;
struct PendingRequest {
PendingRequest *next;
- xcondition_rec condition;
- int waiters; /* Number of threads waiting; -1 if no wait needed */
- unsigned int sequence;
+ unsigned long sequence;
};
typedef struct _X11XCBPrivate {
- struct _XLockPtrs lock_fns;
xcb_connection_t *connection;
PendingRequest *pending_requests;
PendingRequest **pending_requests_tail;
xcb_generic_event_t *next_event;
- const char *request_extra;
- int request_extra_size;
- char *partial_request;
- int partial_request_offset;
+ char *real_bufmax;
char *reply_data;
int reply_length;
int reply_consumed;
+ uint64_t last_flushed;
enum XEventQueueOwner event_owner;
XID next_xid;
} _X11XCBPrivate;
@@ -42,15 +36,4 @@ typedef struct _X11XCBPrivate {
int _XConnectXCB(Display *dpy, _Xconst char *display, char **fullnamep, int *screenp);
void _XFreeX11XCBStructure(Display *dpy);
-/* xcb_lock.c */
-
-int _XCBInitDisplayLock(Display *dpy);
-void _XCBShutdownDisplayLock(Display *dpy);
-
-/* _XGetXCBBuffer and _XPutXCBBuffer calls must be paired and must not
- * be nested. */
-
-void _XGetXCBBuffer(Display *dpy);
-void _XPutXCBBuffer(Display *dpy);
-
#endif /* XXCBINT_H */
diff --git a/src/xcb_io.c b/src/xcb_io.c
index dcbe5a8..830ddb9 100644
--- a/src/xcb_io.c
+++ b/src/xcb_io.c
@@ -5,12 +5,50 @@
#include "locking.h"
#include "Xxcbint.h"
#include <xcb/xcbext.h>
-#include <xcb/xcbxlib.h>
#include <assert.h>
+#include <inttypes.h>
+#include <stdint.h>
#include <stdlib.h>
#include <string.h>
+static void return_socket(void *closure)
+{
+ Display *dpy = closure;
+ LockDisplay(dpy);
+ _XSend(dpy, 0, 0);
+ dpy->bufmax = dpy->buffer;
+ UnlockDisplay(dpy);
+}
+
+static void require_socket(Display *dpy)
+{
+ if(dpy->bufmax == dpy->buffer)
+ {
+ uint64_t sent;
+ int flags = 0;
+ /* if we don't own the event queue, we have to ask XCB
+ * to set our errors aside for us. */
+ if(dpy->xcb->event_owner != XlibOwnsEventQueue)
+ flags = XCB_REQUEST_CHECKED;
+ if(!xcb_take_socket(dpy->xcb->connection, return_socket, dpy,
+ flags, &sent))
+ _XIOError(dpy);
+ /* Xlib uses unsigned long for sequence numbers. XCB
+ * uses 64-bit internally, but currently exposes an
+ * unsigned int API. If these differ, Xlib cannot track
+ * the full 64-bit sequence number if 32-bit wrap
+ * happens while Xlib does not own the socket. A
+ * complete fix would be to make XCB's public API use
+ * 64-bit sequence numbers. */
+ assert(!(sizeof(unsigned long) > sizeof(unsigned int)
+ && dpy->xcb->event_owner == XlibOwnsEventQueue
+ && (sent - dpy->last_request_read >= (UINT64_C(1) << 32))));
+ dpy->xcb->last_flushed = dpy->request = sent;
+ dpy->bufmax = dpy->xcb->real_bufmax;
+ }
+}
+
/* Call internal connection callbacks for any fds that are currently
* ready to read. This function will not block unless one of the
* callbacks blocks.
@@ -68,15 +106,6 @@ static void check_internal_connections(Display *dpy)
}
}
-static void condition_wait(Display *dpy, xcondition_t cv)
-{
- _XPutXCBBuffer(dpy);
- xcb_xlib_unlock(dpy->xcb->connection);
- ConditionWait(dpy, cv);
- xcb_xlib_lock(dpy->xcb->connection);
- _XGetXCBBuffer(dpy);
-}
-
static void call_handlers(Display *dpy, xcb_generic_reply_t *buf)
{
_XAsyncHandler *async, *next;
@@ -105,7 +134,17 @@ static xcb_generic_event_t * wait_or_poll_for_event(Display *dpy, int wait)
return event;
}
-static void process_responses(Display *dpy, int wait_for_first_event, xcb_generic_error_t **current_error, unsigned int current_request)
+/* Widen a 32-bit sequence number into a native-word-size (unsigned long)
+ * sequence number. Treating the comparison as a 1 and shifting it avoids a
+ * conditional branch, and shifting by 16 twice avoids a compiler warning when
+ * sizeof(unsigned long) == 4. */
+static void widen(unsigned long *wide, unsigned int narrow)
+{
+ unsigned long new = (*wide & ~0xFFFFFFFFUL) | narrow;
+ *wide = new + ((unsigned long) (new < *wide) << 16 << 16);
+}
+
+static void process_responses(Display *dpy, int wait_for_first_event, xcb_generic_error_t **current_error, unsigned long current_request)
{
void *reply;
xcb_generic_event_t *event = dpy->xcb->next_event;
@@ -114,13 +153,18 @@ static void process_responses(Display *dpy, int wait_for_first_event, xcb_generi
if(!event && dpy->xcb->event_owner == XlibOwnsEventQueue)
event = wait_or_poll_for_event(dpy, wait_for_first_event);
+ require_socket(dpy);
+
while(1)
{
PendingRequest *req = dpy->xcb->pending_requests;
- assert(!(req && current_request && !XCB_SEQUENCE_COMPARE(req->sequence, <=, current_request)));
- if(event && (!req || XCB_SEQUENCE_COMPARE(event->full_sequence, <=, req->sequence)))
+ unsigned long event_sequence = dpy->last_request_read;
+ if(event)
+ widen(&event_sequence, event->full_sequence);
+ assert(!(req && current_request && !XLIB_SEQUENCE_COMPARE(req->sequence, <=, current_request)));
+ if(event && (!req || XLIB_SEQUENCE_COMPARE(event_sequence, <=, req->sequence)))
{
- dpy->last_request_read = event->full_sequence;
+ dpy->last_request_read = event_sequence;
if(event->response_type != X_Error)
{
/* GenericEvents may be > 32 bytes. In this
@@ -139,7 +183,7 @@ static void process_responses(Display *dpy, int wait_for_first_event, xcb_generi
_XEnq(dpy, (xEvent *) event);
wait_for_first_event = 0;
}
- else if(current_error && event->full_sequence == current_request)
+ else if(current_error && event_sequence == current_request)
{
/* This can only occur when called from
* _XReply, which doesn't need a new event. */
@@ -152,18 +196,9 @@ static void process_responses(Display *dpy, int wait_for_first_event, xcb_generi
free(event);
event = wait_or_poll_for_event(dpy, wait_for_first_event);
}
- else if(req && req->waiters != -1)
+ else if(req && req->sequence == current_request)
{
- if(req->sequence == current_request)
- break;
- if(!current_request && !wait_for_first_event)
- break;
- dpy->xcb->next_event = event;
- req->waiters++;
- assert(req->waiters > 0);
- condition_wait(dpy, &req->condition);
- --req->waiters;
- event = dpy->xcb->next_event;
+ break;
}
else if(req && xcb_poll_for_reply(dpy->xcb->connection, req->sequence, &reply, &error))
{
@@ -192,7 +227,7 @@ static void process_responses(Display *dpy, int wait_for_first_event, xcb_generi
if(xcb_connection_has_error(c))
_XIOError(dpy);
- assert_sequence_less(dpy->last_request_read, dpy->request);
+ assert(XLIB_SEQUENCE_COMPARE(dpy->last_request_read, <=, dpy->request));
assert(!wait_for_first_event);
}
@@ -234,32 +269,61 @@ void _XReadEvents(Display *dpy)
*/
void _XSend(Display *dpy, const char *data, long size)
{
+ static const xReq dummy_request;
+ static char const pad[3];
+ struct iovec vec[3];
+ uint64_t requests;
+ _XExtension *ext;
xcb_connection_t *c = dpy->xcb->connection;
if(dpy->flags & XlibDisplayIOError)
return;
- assert(!dpy->xcb->request_extra);
- dpy->xcb->request_extra = data;
- dpy->xcb->request_extra_size = size;
+ if(dpy->bufptr == dpy->buffer && !size)
+ return;
+
+ /* iff we asked XCB to set aside errors, we must pick those up
+ * eventually. iff there are async handlers, we may have just
+ * issued requests that will generate replies. in either case,
+ * we need to remember to check later. */
+ if(dpy->xcb->event_owner != XlibOwnsEventQueue || dpy->async_handlers)
+ {
+ unsigned int sequence;
+ for(sequence = dpy->xcb->last_flushed; sequence < dpy->request; ++sequence)
+ {
+ PendingRequest *req = malloc(sizeof(PendingRequest));
+ assert(req);
+ req->next = 0;
+ req->sequence = sequence;
+ *dpy->xcb->pending_requests_tail = req;
+ dpy->xcb->pending_requests_tail = &req->next;
+ }
+ }
+ requests = dpy->request - dpy->xcb->last_flushed;
+ dpy->xcb->last_flushed = dpy->request;
- /* give dpy->buffer to XCB */
- _XPutXCBBuffer(dpy);
+ vec[0].iov_base = dpy->buffer;
+ vec[0].iov_len = dpy->bufptr - dpy->buffer;
+ vec[1].iov_base = (caddr_t) data;
+ vec[1].iov_len = size;
+ vec[2].iov_base = (caddr_t) pad;
+ vec[2].iov_len = -size & 3;
- if(xcb_flush(c) <= 0)
- _XIOError(dpy);
+ for(ext = dpy->flushes; ext; ext = ext->next_flush)
+ {
+ int i;
+ for(i = 0; i < 3; ++i)
+ if(vec[i].iov_len)
+ ext->before_flush(dpy, &ext->codes, vec[i].iov_base, vec[i].iov_len);
+ }
- /* get a new dpy->buffer */
- _XGetXCBBuffer(dpy);
+ if(xcb_writev(c, vec, 3, requests) < 0)
+ _XIOError(dpy);
+ dpy->bufptr = dpy->buffer;
+ dpy->last_req = (char *) &dummy_request;
check_internal_connections(dpy);
- /* A straight port of XlibInt.c would call _XSetSeqSyncFunction
- * here. However that does no good: unlike traditional Xlib,
- * Xlib/XCB almost never calls _XFlush because _XPutXCBBuffer
- * automatically pushes requests down into XCB, so Xlib's buffer
- * is empty most of the time. Since setting a synchandler has no
- * effect until after UnlockDisplay returns, we may as well do
- * the check in _XUnlockDisplay. */
+ _XSetSeqSyncFunction(dpy);
}
/*
@@ -268,6 +332,7 @@ void _XSend(Display *dpy, const char *data, long size)
*/
void _XFlush(Display *dpy)
{
+ require_socket(dpy);
_XSend(dpy, 0, 0);
_XEventsQueued(dpy, QueuedAfterReading);
@@ -308,10 +373,18 @@ XID _XAllocID(Display *dpy)
void _XAllocIDs(Display *dpy, XID *ids, int count)
{
int i;
- _XPutXCBBuffer(dpy);
+#ifdef XTHREADS
+ if (dpy->lock)
+ (*dpy->lock->user_lock_display)(dpy);
+ UnlockDisplay(dpy);
+#endif
for (i = 0; i < count; i++)
ids[i] = xcb_generate_id(dpy->xcb->connection);
- _XGetXCBBuffer(dpy);
+#ifdef XTHREADS
+ LockDisplay(dpy);
+ if (dpy->lock)
+ (*dpy->lock->user_unlock_display)(dpy);
+#endif
}
static void _XFreeReplyData(Display *dpy, Bool force)
@@ -325,16 +398,9 @@ static void _XFreeReplyData(Display *dpy, Bool force)
static PendingRequest * insert_pending_request(Display *dpy)
{
PendingRequest **cur = &dpy->xcb->pending_requests;
- while(*cur && XCB_SEQUENCE_COMPARE((*cur)->sequence, <, dpy->request))
+ while(*cur && XLIB_SEQUENCE_COMPARE((*cur)->sequence, <, dpy->request))
cur = &((*cur)->next);
- if(*cur && (*cur)->sequence == dpy->request)
- {
- /* Replacing an existing PendingRequest should only happen once,
- when calling _XReply, and the replaced PendingRequest must
- not have a condition set. */
- assert((*cur)->waiters == -1);
- }
- else
+ if(!*cur || (*cur)->sequence != dpy->request)
{
PendingRequest *node = malloc(sizeof(PendingRequest));
assert(node);
@@ -344,8 +410,6 @@ static PendingRequest * insert_pending_request(Display *dpy)
dpy->xcb->pending_requests_tail = &(node->next);
*cur = node;
}
- (*cur)->waiters = 0;
- xcondition_init(&((*cur)->condition));
return *cur;
}
@@ -367,27 +431,15 @@ Status _XReply(Display *dpy, xReply *rep, int extra, Bool discard)
if(dpy->flags & XlibDisplayIOError)
return 0;
- /* Internals of UnlockDisplay done by hand here, so that we can
- insert_pending_request *after* we _XPutXCBBuffer, but before we
- unlock the display. */
- _XPutXCBBuffer(dpy);
+ _XSend(dpy, 0, 0);
current = insert_pending_request(dpy);
- if(!dpy->lock || dpy->lock->locking_level == 0)
- xcb_xlib_unlock(dpy->xcb->connection);
- if(dpy->xcb->lock_fns.unlock_display)
- dpy->xcb->lock_fns.unlock_display(dpy);
+ /* FIXME: drop the Display lock while waiting?
+ * Complicates process_responses. */
reply = xcb_wait_for_reply(c, current->sequence, &error);
- LockDisplay(dpy);
check_internal_connections(dpy);
process_responses(dpy, 0, &error, current->sequence);
- if(current->waiters)
- { /* The ConditionBroadcast macro contains an if; braces needed here. */
- ConditionBroadcast(dpy, ¤t->condition);
- }
- --current->waiters;
-
if(error)
{
_XExtension *ext;
diff --git a/src/xcb_lock.c b/src/xcb_lock.c
deleted file mode 100644
index 71b2383..0000000
--- a/src/xcb_lock.c
+++ /dev/null
@@ -1,235 +0,0 @@
-/* Copyright (C) 2003-2006 Jamey Sharp, Josh Triplett
- * This file is licensed under the MIT license. See the file COPYING. */
-
-#ifdef HAVE_CONFIG_H
-#include <config.h>
-#endif
-
-#include "Xlibint.h"
-#include "locking.h"
-#include "Xxcbint.h"
-#include <xcb/xcbext.h>
-#include <xcb/xcbxlib.h>
-
-#include <pthread.h>
-
-static void _XCBLockDisplay(Display *dpy)
-{
- if(dpy->xcb->lock_fns.lock_display)
- dpy->xcb->lock_fns.lock_display(dpy);
- if(!dpy->lock || dpy->lock->locking_level == 0)
- xcb_xlib_lock(dpy->xcb->connection);
- if(!(dpy->flags & XlibDisplayIOError))
- _XGetXCBBuffer(dpy);
-}
-
-/* XXX: If you change this function, update _XReply's copy of its guts! */
-static void _XCBUnlockDisplay(Display *dpy)
-{
- if(!(dpy->flags & XlibDisplayIOError))
- {
- _XPutXCBBuffer(dpy);
- assert(dpy->xcb->partial_request == 0);
- assert(xcb_get_request_sent(dpy->xcb->connection) == dpy->request);
-
- /* Traditional Xlib does this in _XSend; see the Xlib/XCB version
- * of that function for why we do it here instead. */
- _XSetSeqSyncFunction(dpy);
- }
-
- if(!dpy->lock || dpy->lock->locking_level == 0)
- xcb_xlib_unlock(dpy->xcb->connection);
- if(dpy->xcb->lock_fns.unlock_display)
- dpy->xcb->lock_fns.unlock_display(dpy);
-}
-
-int _XCBInitDisplayLock(Display *dpy)
-{
- if(!dpy->lock_fns && !(dpy->lock_fns = Xcalloc(1, sizeof(*dpy->lock_fns))))
- return 0;
- dpy->xcb->lock_fns.lock_display = dpy->lock_fns->lock_display;
- dpy->lock_fns->lock_display = _XCBLockDisplay;
- dpy->xcb->lock_fns.unlock_display = dpy->lock_fns->unlock_display;
- dpy->lock_fns->unlock_display = _XCBUnlockDisplay;
- return 1;
-}
-
-void _XCBShutdownDisplayLock(Display *dpy)
-{
- if(dpy->lock_fns) {
- Xfree((char *)dpy->lock_fns);
- dpy->lock_fns = NULL;
- }
-}
-
-void _XGetXCBBuffer(Display *dpy)
-{
- static const xReq dummy_request;
- unsigned int xcb_req = xcb_get_request_sent(dpy->xcb->connection);
- if(xcb_connection_has_error(dpy->xcb->connection))
- _XIOError(dpy);
-
- /* if Xlib has a partial request pending then XCB doesn't know about
- * the current request yet */
- if(dpy->xcb->partial_request)
- ++xcb_req;
-
- assert(XCB_SEQUENCE_COMPARE(xcb_req, >=, dpy->request));
- dpy->request = xcb_req;
-
- dpy->last_req = (char *) &dummy_request;
-}
-
-static size_t request_length(struct iovec *vec)
-{
- /* we have at least part of a request. dig out the length field.
- * note that length fields are always in vec[0]: Xlib doesn't split
- * fixed-length request parts. */
- size_t len;
- assert(vec[0].iov_len >= 4);
- len = ((uint16_t *) vec[0].iov_base)[1];
- if(len == 0)
- {
- /* it's a bigrequest. dig out the *real* length field. */
- assert(vec[0].iov_len >= 8);
- len = ((uint32_t *) vec[0].iov_base)[1];
- }
- return len << 2;
-}
-
-static inline int issue_complete_request(Display *dpy, int veclen, struct iovec *vec)
-{
- xcb_protocol_request_t xcb_req = { 0 };
- unsigned int sequence;
- int flags = XCB_REQUEST_RAW;
- int i;
- size_t len;
-
- /* skip empty iovecs. if no iovecs remain, we're done. */
- assert(veclen >= 0);
- while(veclen > 0 && vec[0].iov_len == 0)
- --veclen, ++vec;
- if(!veclen)
- return 0;
-
- len = request_length(vec);
-
- /* do we have enough data for a complete request? how many iovec
- * elements does it span? */
- for(i = 0; i < veclen; ++i)
- {
- size_t oldlen = len;
- len -= vec[i].iov_len;
- /* if len is now 0 or has wrapped, we have enough data. */
- if((len - 1) > oldlen)
- break;
- }
- if(i == veclen)
- return 0;
-
- /* we have enough data to issue one complete request. the remaining
- * code can't fail. */
-
- /* len says how far we overshot our data needs. (it's "negative" if
- * we actually overshot, or 0 if we're right on.) */
- vec[i].iov_len += len;
- xcb_req.count = i + 1;
- xcb_req.opcode = ((uint8_t *) vec[0].iov_base)[0];
-
- /* if we don't own the event queue, we have to ask XCB to set our
- * errors aside for us. */
- if(dpy->xcb->event_owner != XlibOwnsEventQueue)
- flags |= XCB_REQUEST_CHECKED;
-
- /* XCB will always skip request 0; account for that in the Xlib count */
- if (xcb_get_request_sent(dpy->xcb->connection) == 0xffffffff)
- dpy->request++;
- /* send the accumulated request. */
- sequence = xcb_send_request(dpy->xcb->connection, flags, vec, &xcb_req);
- if(!sequence)
- _XIOError(dpy);
-
- /* update the iovecs to refer only to data not yet sent. */
- vec[i].iov_len = -len;
-
- /* iff we asked XCB to set aside errors, we must pick those up
- * eventually. iff there are async handlers, we may have just
- * issued requests that will generate replies. in either case,
- * we need to remember to check later. */
- if(flags & XCB_REQUEST_CHECKED || dpy->async_handlers)
- {
- PendingRequest *req = malloc(sizeof(PendingRequest));
- assert(req);
- req->next = 0;
- req->waiters = -1;
- req->sequence = sequence;
- *dpy->xcb->pending_requests_tail = req;
- dpy->xcb->pending_requests_tail = &req->next;
- }
- return 1;
-}
-
-void _XPutXCBBuffer(Display *dpy)
-{
- static char const pad[3];
- const int padsize = -dpy->xcb->request_extra_size & 3;
- xcb_connection_t *c = dpy->xcb->connection;
- _XExtension *ext;
- struct iovec iov[6];
-
- assert_sequence_less(dpy->last_request_read, dpy->request);
- assert_sequence_less(xcb_get_request_sent(c), dpy->request);
-
- for(ext = dpy->flushes; ext; ext = ext->next_flush)
- {
- ext->before_flush(dpy, &ext->codes, dpy->buffer, dpy->bufptr - dpy->buffer);
- if(dpy->xcb->request_extra)
- {
- ext->before_flush(dpy, &ext->codes, dpy->xcb->request_extra, dpy->xcb->request_extra_size);
- if(padsize)
- ext->before_flush(dpy, &ext->codes, pad, padsize);
- }
- }
-
- iov[2].iov_base = dpy->xcb->partial_request;
- iov[2].iov_len = dpy->xcb->partial_request_offset;
- iov[3].iov_base = dpy->buffer;
- iov[3].iov_len = dpy->bufptr - dpy->buffer;
- iov[4].iov_base = (caddr_t) dpy->xcb->request_extra;
- iov[4].iov_len = dpy->xcb->request_extra_size;
- iov[5].iov_base = (caddr_t) pad;
- iov[5].iov_len = padsize;
-
- while(issue_complete_request(dpy, 4, iov + 2))
- /* empty */;
-
- /* first discard any completed partial_request. */
- if(iov[2].iov_len == 0 && dpy->xcb->partial_request)
- {
- free(dpy->xcb->partial_request);
- dpy->xcb->partial_request = 0;
- dpy->xcb->partial_request_offset = 0;
- }
-
- /* is there anything to copy into partial_request? */
- if(iov[3].iov_len != 0 || iov[4].iov_len != 0 || iov[5].iov_len != 0)
- {
- int i;
- if(!dpy->xcb->partial_request)
- {
- size_t len = request_length(iov + 3);
- assert(!dpy->xcb->partial_request_offset);
- dpy->xcb->partial_request = malloc(len);
- assert(dpy->xcb->partial_request);
- }
- for(i = 3; i < sizeof(iov) / sizeof(*iov); ++i)
- {
- memcpy(dpy->xcb->partial_request + dpy->xcb->partial_request_offset, iov[i].iov_base, iov[i].iov_len);
- dpy->xcb->partial_request_offset += iov[i].iov_len;
- }
- }
-
- dpy->xcb->request_extra = 0;
- dpy->xcb->request_extra_size = 0;
- dpy->bufptr = dpy->buffer;
-}
--
1.5.6.5
From d4e77ee5023cda7cb04afdce9be81eaf72785c2a Mon Sep 17 00:00:00 2001
From: Jamey Sharp <jamey@minilop.net>
Date: Sun, 23 Mar 2008 16:33:50 -0700
Subject: [PATCH] Fix XAllocID race: hold the user display lock until we have a new XID.
Xlib built --without-xcb is also vulnerable to this race, and a similar
fix might work there too.
Also, use an XID that's truly invalid while waiting for the next XID to be
requested.
---
src/xcb_io.c | 11 ++++++++++-
1 files changed, 10 insertions(+), 1 deletions(-)
diff --git a/src/xcb_io.c b/src/xcb_io.c
index 830ddb9..d0abf4c 100644
--- a/src/xcb_io.c
+++ b/src/xcb_io.c
@@ -343,6 +343,10 @@ _XIDHandler(Display *dpy)
{
XID next = xcb_generate_id(dpy->xcb->connection);
LockDisplay(dpy);
+#ifdef XTHREADS
+ if (dpy->lock)
+ (*dpy->lock->user_unlock_display)(dpy);
+#endif
dpy->xcb->next_xid = next;
if(dpy->flags & XlibDisplayPrivSync)
{
@@ -357,8 +361,13 @@ _XIDHandler(Display *dpy)
/* _XAllocID - resource ID allocation routine. */
XID _XAllocID(Display *dpy)
{
+ const XID inval = ~0UL;
XID ret = dpy->xcb->next_xid;
- dpy->xcb->next_xid = 0;
+#ifdef XTHREADS
+ if (ret != inval && dpy->lock)
+ (*dpy->lock->user_lock_display)(dpy);
+#endif
+ dpy->xcb->next_xid = inval;
if(!(dpy->flags & XlibDisplayPrivSync))
{
--
1.5.6.5
From 1cfce69bf65d9631a72f8b20d637bf158c0afd20 Mon Sep 17 00:00:00 2001
From: Keith Packard <keithp@keithp.com>
Date: Sat, 11 Oct 2008 21:10:23 -0700
Subject: [PATCH] Permit only one Xlib thread to block waiting for events
As Xlib queues events internally, we must prevent multiple Xlib threads from
entering XCB to wait for an event in case the queued event is to be
delivered to the thread which didn't manage to read it. In other words, let
only one Xlib thread into xcb_wait_for_event at a time.
Jamey Sharp looked over my shoulder while making this fix and, while hating
my whitespace conventions, appears happy enough with the actual code.
Signed-off-by: Keith Packard <keithp@keithp.com>
---
src/Xxcbint.h | 7 +++++++
src/xcb_disp.c | 2 ++
src/xcb_io.c | 18 ++++++++++++++----
3 files changed, 23 insertions(+), 4 deletions(-)
diff --git a/src/Xxcbint.h b/src/Xxcbint.h
index 1b3137b..f4af324 100644
--- a/src/Xxcbint.h
+++ b/src/Xxcbint.h
@@ -8,6 +8,7 @@
#include <stdint.h>
#include <X11/Xlibint.h>
#include <X11/Xlib-xcb.h>
+#include "locking.h"
#define XLIB_SEQUENCE_COMPARE(a,op,b) (((long) (a) - (long) (b)) op 0)
@@ -29,6 +30,12 @@ typedef struct _X11XCBPrivate {
uint64_t last_flushed;
enum XEventQueueOwner event_owner;
XID next_xid;
+
+ /* handle simultaneous threads waiting for events,
+ * used in wait_or_poll_for_event
+ */
+ xcondition_t event_notify;
+ int event_waiter;
} _X11XCBPrivate;
/* xcb_disp.c */
diff --git a/src/xcb_disp.c b/src/xcb_disp.c
index dfd6803..628c654 100644
--- a/src/xcb_disp.c
+++ b/src/xcb_disp.c
@@ -93,6 +93,7 @@ int _XConnectXCB(Display *dpy, _Xconst char *display, char **fullnamep, int *scr
dpy->xcb->pending_requests_tail = &dpy->xcb->pending_requests;
dpy->xcb->next_xid = xcb_generate_id(dpy->xcb->connection);
+ dpy->xcb->event_notify = xcondition_malloc();
return !xcb_connection_has_error(c);
}
@@ -106,5 +107,6 @@ void _XFreeX11XCBStructure(Display *dpy)
dpy->xcb->pending_requests = tmp->next;
free(tmp);
}
+ xcondition_free(dpy->xcb->event_notify);
Xfree(dpy->xcb);
}
diff --git a/src/xcb_io.c b/src/xcb_io.c
index d0abf4c..fc8e53f 100644
--- a/src/xcb_io.c
+++ b/src/xcb_io.c
@@ -125,9 +125,20 @@ static xcb_generic_event_t * wait_or_poll_for_event(Display *dpy, int wait)
xcb_generic_event_t *event;
if(wait)
{
- UnlockDisplay(dpy);
- event = xcb_wait_for_event(c);
- LockDisplay(dpy);
+ if(dpy->xcb->event_waiter)
+ {
+ ConditionWait(dpy, dpy->xcb->event_notify);
+ event = xcb_poll_for_event(c);
+ }
+ else
+ {
+ dpy->xcb->event_waiter = 1;
+ UnlockDisplay(dpy);
+ event = xcb_wait_for_event(c);
+ LockDisplay(dpy);
+ dpy->xcb->event_waiter = 0;
+ ConditionBroadcast(dpy, dpy->xcb->event_notify);
+ }
}
else
event = xcb_poll_for_event(c);
@@ -228,7 +239,6 @@ static void process_responses(Display *dpy, int wait_for_first_event, xcb_generi
_XIOError(dpy);
assert(XLIB_SEQUENCE_COMPARE(dpy->last_request_read, <=, dpy->request));
- assert(!wait_for_first_event);
}
int _XEventsQueued(Display *dpy, int mode)
--
1.5.6.5
From bbeafc850181cc0bf9724c71f1aa0015acdb7480 Mon Sep 17 00:00:00 2001
From: Keith Packard <keithp@keithp.com>
Date: Sat, 11 Oct 2008 21:44:21 -0700
Subject: [PATCH] Ensure that _XReadEvents always leaves an event in the queue on return
XNextEvent assumes that the event queue will be non-empty on return from
_XReadEvents, but with multiple event readers running, the previous change
could leave the queue empty on return from process_responses. Re-invoke
process_responses until the queue is non-empty.
Signed-off-by: Keith Packard <keithp@keithp.com>
---
src/xcb_io.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/src/xcb_io.c b/src/xcb_io.c
index fc8e53f..2256a3f 100644
--- a/src/xcb_io.c
+++ b/src/xcb_io.c
@@ -267,7 +267,9 @@ void _XReadEvents(Display *dpy)
if(dpy->xcb->event_owner != XlibOwnsEventQueue)
return;
check_internal_connections(dpy);
- process_responses(dpy, 1, 0, 0);
+ do {
+ process_responses(dpy, 1, 0, 0);
+ } while (dpy->qlen == 0);
}
/*
--
1.5.6.5
Attachment:
signature.asc
Description: Digital signature