Broken XNextEvent()(libX11 1.1.2 - 1.1.5)

[Fumihito YOSHIDA]

Hi lists,

In libX11 1.1.2 (and newer), XNextEvent() sometimes segfault by null
exception.

This problem look like come from _XReadEvents()/_XSend() behavior.
The problem is not reproduce in 1.1.1 and old.

Compiz and IIIMF fall under this problem, but these are typically
example, I think that many programs stuck by this problem.

Is it bug, or new specification?

=====================
Details:
=====================

A new implementation of _XReadEvents() is lockless, and compatible
for olddays behavior, but it cause segfault at some conditions.

(_XReadEvents() take least one events, but when XLostDisplay flag is
 true, _XReadEvents() does not returned any valid events.
So, it cause segfault in XNextEvent() by null pointer exception.),

For example, Florian Echtler said in below mail,
http://lists.freedesktop.org/archives/xorg/2008-September/038344.html
this crash is typicial pattern of this bug.

This is *not* coding issue, but you can evade this problem with some
coding design.

Case1: Classical X event loop
 // It will cause segfault in some conditions....

Display *pdisplay;
XEvent event;
pdisplay = XOpenDisplay(NULL);

for (; ;){
	XNextEvent(pdisplay, &event);
	/* X Event Handler */
}

This code (rarely) causes segfault at XNextEvent().
If you trace it, you will find below crash point.

--- XNextEvent() ----------------------------------
int
XNextEvent (
        register Display *dpy,
        register XEvent *event)
{
        register _XQEvent *qelt;

        LockDisplay(dpy);

        if (dpy->head == NULL)
            _XReadEvents(dpy);  // <- it hope _XReadEvents return valid
                                       //  handler, it believev
dpy->head != null...
        qelt = dpy->head;
        *event = qelt->event; // <- but, dpy->head->event is null, CRASH.
                                      // Because dpy->head is null!
        _XDeq(dpy, NULL, qelt);
        UnlockDisplay(dpy);
        return 0;
}
--------------------------------------------------

If you replace "if(dpy->head == null)" to "while(dpy->head == null)",
the problem solved.
// Notice: if => while replacement cause busy loop and possible infinite loop.

This segfaults conditions are:
  a) X Event queue is empty.
  b) XDisplayIOError is true.
You call XNextEvent() when (a && b), your program will segfault.


Case2: Modern X event loop style is below:
 // It wont cause segfault, and lockless.

Display *pdisplay;
XEvent event;
pdisplay = XOpenDisplay(NULL);

for (; ;){
	if (XPending()){
		XNextEvent(pdisplay, &event);
		/* X Event Handler */
	}
	/* other event looping and some usleeps. */
}

Because this implement want evading XNextEvent()'s blocking.
XNextEvent() always return least one events, but when X event
queue is empty, cause blocking-wait for next new events.

But, man XNextEvent(3) says:
|  The XNextEvent function copies the first event from the event queue
|  into the specified XEvent structure and then removes it from the queue.
|  If the event queue is empty, XNextEvent flushes the output buffer and
|  blocks until an event is received.

Current implement is not "blocks", "sometimes blocks, sometimes crash".

And we have big issue, the classical X event loop(Case1) are not failure
in old-days, their coding were(are?) right. (But, their old style coding are not
efficient, we need modern style.).

We can find their classical event loop in many code asset, they will crash
by this problem.....

=====================
Root cause:
=====================
It cause by xcb_io.c::_XReadEvents() and xcb_io.c::_XSend().

When dpy->head is null, the XNextEvent() called _XReadEvents(),
but xcb_io.c::_XReadEvents() and xcb_io.c::_XSend() has dpy->flags check,
when XlibDiskpayIOError bit is set, their procedure return without any work.

        if(dpy->flags & XlibDisplayIOError)
                return;

So, XNextEvent()'s dpy is still null, and crash.

I suggest dirty hack.
 XlibDisplayIOError flag can be cleard with some wait times.

--- NextEvent.c.orig
+++ NextEvent.c
@@ -45,9 +45,14 @@
        register _XQEvent *qelt;

 	LockDisplay(dpy);

-	if (dpy->head == NULL)
+        while(dpy->head == NULL){
            _XReadEvents(dpy);
+           usleep(400);
+	}
 	qelt = dpy->head;
 	*event = qelt->event;
 	_XDeq(dpy, NULL, qelt);


Regards,