Broken XNextEvent()(libX11 1.1.2 - 1.1.5)

[walter harms]

mmh,
i am not an expenrt on xorg internal but why not add
if (!XPending()) return; to XNextEvent(); ?

that would translate old stile loop into the new form
and leave other programs with 2 Pending() calls, what
should be harmless. in the long run the if would eliminate
the if XPending() from all the loops in the current programs.

re
 wh



Fumihito YOSHIDA schrieb:
> 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,
> _______________________________________________
> xorg mailing list
> xorg at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/xorg
> 
>