Bug 8622

Summary:	libX11/XCB threads: simultaneous event-wait with other Xlib request breaks
Product:	xorg	Reporter:	Diego Elio Pettenò <flameeyes>
Component:	Lib/Xlib	Assignee:	Jamey Sharp <jamey>
Status:	RESOLVED FIXED	QA Contact:
Severity:	major
Priority:	high	CC:	josh, x
Version:	unspecified
Hardware:	All
OS:	All
Whiteboard:
i915 platform:		i915 features:
Bug Depends on:
Bug Blocks:	6666

Description Diego Elio Pettenò 2006-10-12 16:44:06 UTC

Testing XCB on Gentoo Linux (Xorg 7.1, libX11 1.0.99.1-r1), I found that 
Kaffeine (http://kaffeine.sf.net) is crashing down as soon as xine is 
initialised. The problem seems to be at 
kaffeine-0.8.2/kaffeine/src/player-parts/xine-part/kxinewidget.cpp:1044:

                XNextEvent (m_xineDisplay, &event);

this causes a NULL pointer dereference, because when calling XNextEvent, qelt 
(libX11-1.0.99.1/src/NextEvent.c:51) is still NULL:

49        if (dpy->head == NULL)
50            _XReadEvents(dpy);
51        qelt = dpy->head;
52        *event = qelt->event;

line 52 is where the NULL pointer dereference actually is.

Why this happens, seems to be in _XReadEvents 
(libX11-1.0.99.1/src/xcb_io.c:176):

175        _XSend(dpy, 0, 0);
176        if(dpy->xcb->event_owner != XlibOwnsEventQueue)
177                return;
178        check_internal_connections(dpy);
179        process_responses(dpy, 1, 0, dpy->request);

but I cannot be sure of this because when I tried to put a breakpoint on line 
176 GDB left me for a sexier segfault...

Everything works fine with a non-xcb-enabled libX11.

Comment 1 Jamey Sharp 2006-11-11 16:45:00 UTC

Sorry I haven't responded to this bug. I looked at it for a little while and was
baffled. I'm getting back to it now and hope to have an answer/fix for you soon.

I gather you were able to reliably reproduce this bug? Just to be sure, can you
reproduce it now, and also with a current git clone of libX11?

I also wonder if Kaffeine is multi-threaded, as I've thought there might be a
race in process_responses...

Comment 2 Diego Elio Pettenò 2006-11-11 16:52:16 UTC

Not sure if Kaffeine itself is threaded (could check but it's late now and I'm 
feverish), but xine is for sure.
And yes, this can be reproduced with two days ago's git version (will confirm 
tomorrow with very current ones).

Comment 3 Jamey Sharp 2006-11-17 00:29:54 UTC

OK, now I know what's wrong, but I haven't decided how to fix it yet. It's a
threading bug, and the following patch masks the problem. You could apply it if
you want until we get a real fix in: I've verified that kaffeine will play stuff
happily with the patch applied.

I'm making this a blocker for 7.2, as it's a race condition that affects any of
the three programs in the world that actually make Xlib calls from more than one
thread.

diff --git a/src/xcb_io.c b/src/xcb_io.c
index 7717679..7a038d9 100644
--- a/src/xcb_io.c
+++ b/src/xcb_io.c
@@ -106,6 +106,8 @@ static void process_responses(Display *d
                        UnlockDisplay(dpy);
                        event = xcb_wait_for_event(c);
                        LockDisplay(dpy);
+                       /* FIXME: need equivalent of reply/event awaiters CV
lists here, not this hack */
+                       current_request = dpy->request;
                }
                else
                        event = xcb_poll_for_event(c);

Exercise for the reader: identify why adding this one line helps. Bonus
question: explain why it's the wrong fix. Extra bonus points: provide the right fix.

Comment 4 Jamey Sharp 2006-11-18 15:56:57 UTC

After extensive root cause analysis, a lot of thought, and about 80 new lines of
code, Josh and I believe we have correctly fixed this bug without introducing
regressions.

When you get a chance, please re-test with a current git checkout of libX11. Thanks!

Comment 5 Diego Elio Pettenò 2006-11-19 06:16:09 UTC

Tested this last night, and now it works as a charm (on FreeBSD with the 
libxcb patch), thanks!

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.