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

[Jamey Sharp]

Sorry for not replying sooner; my bad!

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.

I'd prefer this not go into a release until we have evidence that it's a
good idea.

Jamey

On Sat, Jun 01, 2013 at 10:33:01PM +0200, Uli Schlachter wrote:
> On 15.05.2013 11:21, Christian König wrote:
> > From: Christian König <christian.koenig at amd.com>
> > 
> > To prevent different threads from stealing the socket from each other the
> > caller of "xcb_take_socket" must hold a lock that is also acquired in
> > "return_socket". Unfortunately xcb tries to prevent calling return_socket
> > from multiple threads and this can lead to a deadlock situation.
> > 
> > A simple example:
> > - X11 has taken the socket
> > - Thread A has locked the display.
> > - Thread B does xcb_no_operation() and thus ends up in libX11's return_socket(),
> >   waiting for the display lock.
> > - Thread A calls e.g. xcb_no_operation(), too, ends up in return_socket() and
> >   because socket_moving == 1, ends up waiting for thread B
> > => Deadlock
> > 
> > This patch allows calling return_socket from different threads at the same time
> > an so resolves the deadlock situation.
> > 
> > Partially fixes: https://bugs.freedesktop.org/show_bug.cgi?id=20708
> > 
> > v2: fixes additional pthread_cond_wait dependencies,
> >     rework comments and patch description
> > 
> > v3: separate pthread_cond_wait dependencies and unrelated whitespace
> >     change into their own patch, use unsigned for socket_seq
> > 
> > Signed-off-by: Christian König <christian.koenig at amd.com>
> 
> Since no one spoke up and complained (or said anything at all...):
> Thanks, I just pushed this patch. Let's see if this motivates anyone to complain
> now or if this patch ends up in the next release.
> 
> Cheers,
> Uli
> 
> P.S.: Let's see how long it will take patch 3/3 to get in. It certainly does
> improve the current state, but my head starts spinning when I wonder if it fixes
> the whole issue.
> -- 
> Sent from my Game Boy.
> _______________________________________________
> Xcb mailing list
> Xcb at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/xcb
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.freedesktop.org/archives/xcb/attachments/20130605/f836fc53/attachment.pgp>