[PATCH xlib] Add support for a display-specific error handler

[Jasper St. Pierre]

Yes. This is the case we're seeing.

Looking back on it now, it seems my patch doesn't actually help,
because my driver wasn't always using a separate Display connection
like I thought it was. TLS doesn't actually help, since the main-loop
is always in the same situation. I wonder if this would work:

   XLockDisplay();
   old = XSetErrorHandler(my_handler);
   Request1();
   Request2();
   Request3();
   XSync();
   XSetErrorHandler(old);
   /* check for errors */
   XUnlockDisplay();

That would then become a critical section along the display, right?
Alternatively, XGrabServer, but that's a bit weightier.

On Mon, Nov 16, 2015 at 1:11 PM, Adam Jackson <ajax at nwnk.net> wrote:
> On Fri, 2015-11-13 at 15:24 -0800, Jasper St. Pierre wrote:
>> Writing error-safe code that uses Xlib is too obnoxious, and using XCB
>> is tedious and not performant, as we can't catch events on a giant
>> stream -- we have to check every operation manually.
>>
>> In my specific case (writing a GL driver), it would be enough to simply
>> have a Display-specific error handler, since our driver re-opens the
>> Display in our own code, but a better approach might be TLS storage for
>> the global handler.
>
> As I noted on IRC, using an "internal" display connection like this is
> playing with fire and I'd suggest not doing it, but that's sort of
> orthogonal to the patch itself.
>
> I'm not really sure what this new API wins you, though. Installing an
> error handler is thread-safe thanks to _Xglobal_lock, and the handler
> is called with the affected display as the first argument (_and_
> stashed in the XError), so you've got all the information you need to
> inspect the event and handle it iff it's yours and pass it through to
> the old handler.
>
> Though, now that I've said that, technically push/popping the display
> hander like this isn't safe if you can't control the order of the calls
> to SetErrorHandler:
>
> A: "push" SetErrorHandler(a) returns NULL
> B: "push" SetErrorHandler(b) returns a
> A: "pop" SetErrorHandler(NULL) returns b
> B: "pop" SetErrorHandler(a) returns b
>
> and now you have thread A's handler installed, when you were trying to
> get back to normal.
>
> - ajax



-- 
  Jasper