[Xcb] xpyb and threading

[Josh Triplett]

On Sat, Feb 20, 2010 at 04:15:07PM +0100, Ander Martinez wrote:
> Hello, this is the first time I write to this list.
> I've been writing some stuff using xpyb. I have multiple threads in
> python and one of them is using wait_for_event which caused the whole
> process (the interpreter and therefore the other threads) to block.
> Some investigation brought me to xpyb/src/conn.c where sandwiching the
> line 'data = xcb_wait_for_event(self->conn);' between
> Py_BEGIN_ALLOW_THREADS and Py_END_ALLOW_THREADS solved my issue. I've
> included a patch.
> 
> Also, I think that most blocking functions should unlock the GIL, but
> I haven't patched that.

> --- conn.old.c	2010-02-20 16:09:33.376127518 +0100
> +++ conn.c	2010-02-20 16:09:37.739110218 +0100
> @@ -429,8 +429,10 @@
>  
>      if (xpybConn_invalid(self))
>  	return NULL;
> -
> +    
> +    Py_BEGIN_ALLOW_THREADS
>      data = xcb_wait_for_event(self->conn);
> +    Py_END_ALLOW_THREADS
>  
>      if (data == NULL) {
>  	PyErr_SetString(PyExc_IOError, "I/O error on X server connection.");

In general, I think it makes sense to drop the GIL when calling XCB
functions.  However, I see two issues with this patch.

First, while I don't claim to have any expertise with the Python C API,
as I understand it you must not access Python objects unless you hold
the GIL; here, you use self->conn, but can another thread make self go
away before you dereference it?  For that matter, can another thread
call xcb_disconnect on self->conn before you call xcb_wait_for_event?
Both of those things would make this thread access freed memory.

Second, you've added trailing whitespace in your patch; don't do that. :)

- Josh Triplett