[Xcb] [PATCH 2/3] fix deadlock with xcb_take_socket/return_socket v3

[Josh Triplett]

On Mon, Jun 10, 2013 at 12:45:12PM +0200, Christian König wrote:
> Am 07.06.2013 18:11, schrieb Josh Triplett:
> >On Fri, Jun 07, 2013 at 04:48:46PM +0200, Uli Schlachter wrote:
> >>On 05.06.2013 22:51, Jamey Sharp wrote:
> >>>I think this patch correctly does what it claims to do, which is always
> >>>a plus. :-) I haven't thought carefully about it, but it looks
> >>>plausible.
> >>>
> >>>However, I have yet to see evidence that this patch fixes a bug. I
> >>>believe instead that it works around broken callers, which I'd rather
> >>>discourage.
> >>Well, the only broken caller that was considered so far was "something calls
> >>into xcb via XGetXCBConnection() while the Display* is locked", right? You are
> >>saying that the caller shouldn't do this and has to be fixed instead.
> >>
> >>However, how could such a fix look like? This rule would boil down to "you must
> >>not use XGetXCBConnection() in a library", because anyone up the call-chain
> >>could have used XLockDisplay(). Thus, XGetXCBConnection() and libxcb would
> >>become unusable for almost anyone (perhaps except for toolkits).
> >I'd argue that the fix belongs in the caller here, not the library:
> >"don't expect random X libraries to work when you call them with the
> >display lock held".  Though I do agree that such a change could break
> >code which worked with Xlib, which does seem problematic.
> 
> Well, after looking into the xbmc (the application which leads to
> the discovery of this problem) code in question the problem seems to
> be even harder to solve. The comment there states that with the
> closed source drivers it is *necessary* to have the display locked
> while creating the VDPAU device to avoid problems.

I have a hard time imagining what could have led to that conclusion.  I
could imagine a requirement like "don't create VDPAU devices
simultaneously from multiple threads"; that much still seems broken, but
in an Xlib kind of way.  But the X display lock specifically?

In any case, sane or not, it's something that existing programs do. :(

> Unfortunately that works only with the NVidia closed source driver
> (since it uses Xlib), but not with the open source implementation
> found in mesa (which uses XCB).
> 
> So the argument that we should fix the caller instead doesn't seem
> to work, cause in one case we have mesas implementation of
> "glFlush()" which definitely shouldn't be called with the display
> lock held, and on the other hand we have VDPAU which *must* be
> called with the display lock held.
> 
> So in the end it is impossible to fix this behavior in the
> application while maintaining compatibility with the binary only
> drivers.

Still skeptical of the supposed VDPAU requirement, but I don't fancy
fighting that particular battle.

- Josh Triplett