[Xcb] xpyb and threading

[Eamon Walsh]

Josh Triplett wrote:
> 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.
>   

It looks like it is possible for this to race with xcb_disconnect() if
another thread calls the "disconnect" method explicitly.

I think the only way to fix this is by adding locks to protect against
it.  An alternative would be to remove the disconnect method from xpyb
or turn it into a no-op, but that would make it impossible to close the
connection immediately (it would happen at garbage collection time).


--Eamon W.