From be0fe56c3bcad5124dcc6c47a2fad01acd16f71a Mon Sep 17 00:00:00 2001 From: Keith Packard Date: Mon, 23 Dec 2013 21:15:20 -0800 Subject: [PATCH] Ensure xcb owns socket and no other threads are writing before send_request send_request may only write to out.queue if no other thread is busy writing to the network (as that thread may be writing from out.queue). send_request may only allocate request sequence numbers if XCB owns the socket. Therefore, send_request must make sure that both conditions are true when it holds iolock, which can only be done by looping until both conditions are true without having dropped the lock waiting for the second condition. We choose to get the socket back from Xlib first as get_socket_back has a complicated test and checking for other threads writing is a simple in-lined check. This also changes the sequence number checks (64k requests with no reply, 4M request wrapping) to ensure that both conditions are true before queueing the request. Signed-off-by: Keith Packard Reviewed-by: Uli Schlachter --- src/xcb_out.c | 57 ++++++++++++++++++++++++++++++++++++++++----------------- 1 file changed, 40 insertions(+), 17 deletions(-) diff --git a/src/xcb_out.c b/src/xcb_out.c index 18bb5f9..dc42954 100644 --- a/src/xcb_out.c +++ b/src/xcb_out.c @@ -103,6 +103,33 @@ static void get_socket_back(xcb_connection_t *c) _xcb_in_replies_done(c); } +static void prepare_socket_request(xcb_connection_t *c) +{ + /* We're about to append data to out.queue, so we need to + * atomically test for an external socket owner *and* some other + * thread currently writing. + * + * If we have an external socket owner, we have to get the socket back + * before we can use it again. + * + * If some other thread is writing to the socket, we assume it's + * writing from out.queue, and so we can't stick data there. + * + * We satisfy this condition by first calling get_socket_back + * (which may drop the lock, but will return when XCB owns the + * socket again) and then checking for another writing thread and + * escaping the loop if we're ready to go. + */ + for (;;) { + if(c->has_error) + return; + get_socket_back(c); + if (!c->out.writing) + break; + pthread_cond_wait(&c->out.cond, &c->iolock); + } +} + /* Public interface */ void xcb_prefetch_maximum_request_length(xcb_connection_t *c) @@ -236,24 +263,23 @@ unsigned int xcb_send_request(xcb_connection_t *c, int flags, struct iovec *vect /* get a sequence number and arrange for delivery. */ pthread_mutex_lock(&c->iolock); - /* 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); + + prepare_socket_request(c); /* send GetInputFocus (sync_req) when 64k-2 requests have been sent without - * a reply. */ - if(req->isvoid && c->out.request == c->in.request_expected + (1 << 16) - 2) - send_sync(c); - /* Also send sync_req (could use NoOp) at 32-bit wrap to avoid having + * a reply. + * Also send sync_req (could use NoOp) at 32-bit wrap to avoid having * applications see sequence 0 as that is used to indicate - * an error in sending the request */ - if((unsigned int) (c->out.request + 1) == 0) + * an error in sending the request + */ + + while ((req->isvoid && c->out.request == c->in.request_expected + (1 << 16) - 2) || + (unsigned int) (c->out.request + 1) == 0) + { send_sync(c); + prepare_socket_request(c); + } - /* The above send_sync calls could drop the I/O lock, but this - * thread will still exclude any other thread that tries to write, - * so the sequence number postconditions still hold. */ send_request(c, req->isvoid, workaround, flags, vector, veclen); request = c->has_error ? 0 : c->out.request; pthread_mutex_unlock(&c->iolock); @@ -373,10 +399,7 @@ int _xcb_out_send(xcb_connection_t *c, struct iovec *vector, int count) void _xcb_out_send_sync(xcb_connection_t *c) { - /* 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); + prepare_socket_request(c); send_sync(c); } -- 1.9.1