[RFC PATCHv2] New XCB socket handoff mechanism for Xlib/XCB and other libraries

[pcpa at mandriva.com.br]

Quoting Colin Guthrie <gmane at colin.guthr.ie>:

> Jamey Sharp wrote:
>> On Sat, Mar 15, 2008 at 09:36:37PM -0700, 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.
>>
>> After reviewing and testing these patches for the last week, we shook
>> out a few small bugs.  The attached patches represent version 2 of the
>> handoff implementation, which fixes most of the bugs we're aware of.  As
>> before, when applying the Xlib/XCB patches, please make sure you use
>> current Xlib from Git or have the patch from Git commit
>> 64325f38bab082a8e0e9ce779a8e582de5c8588e applied.
>>
>> Changes to the XCB side since version 1 of the handoff implementation:
>>
>> * Fix bad invariant asserts in _xcb_in_replies_done: external owners
>>   will not have a pending_reply unless they set default flags.
>> * Fix missed wakeup bug in get_socket_back by using
>>   pthread_cond_broadcast rather than pthread_cond_signal.
>> * Use _xcb_in_expect_reply rather than adding a new function
>>   _xcb_in_expect_replies that does almost the same thing.
>>
>> On the Xlib/XCB side:
>>
>> * Fix a race condition in the way XAllocID allocates XIDs.  Previously,
>>   if a thread allocated an XID with XAllocID, another thread could run
>>   between the subsequent UnlockDisplay and SyncHandle, before the
>>   _XIDHandler obtained another XID.  Prevent this by holding the user
>>   display lock between XAllocID and the subsequent SyncHandle.
>>
>>   This race condition also exists in traditional Xlib, and a similar fix
>>   should work there too.
>>
>> * Use an XID that's truly invalid as a placeholder before requesting a
>>   new XID.  Previously, XID allocation used 0 as a placeholder, but that
>>   represents a potentially valid XID.
>>
>> - Josh Triplett and Jamey Sharp
>
> Was this ever officially accepted/committed?
>
> I see it on:
> http://gitweb.freedesktop.org/?p=users/cloos/libxcb.git;a=summary
> and
> http://gitweb.freedesktop.org/?p=users/cloos/libX11.git;a=summary
>
> but is this planned to be merged to master on the official repos?
>
> I want to update the Mandriva cooker package but as a lot of other
> packages link against libxcb-xlib and will require rebuilt, I don't want
> to push it out only to find that this wont actually happen and I'll have
> to rebuild everything again :s

  You may want to James about if/when it will be updated to git master,
but I believe it is in his private branch due to not having yet being
tested enough.

  (Related to your ask for a comment in bugzilla, but only responding
here for now :-)) Maybe it is also a problem that can be corrected in
the vmware client itself. I remember some days ago helping with a
related problem, where a "separated" thread did Xlib calls, but
reorganizing the code to only make those calls from the main thread
fixed the problem...

> Col

Paulo