Publicly expose last error received from X11

[kchibisov]

Unfortunately in multi-threaded environment there's no way to handle errors safely other than setting error handling routine and store the error information in some thread safe structure like we do in winit with mutex.

However the problem with xlib here is that error handling callback is global for the entire process, so if glutin wants to make use of GLX, but doesn't want to depend on winit, it must get the error information out of winit somehow(glutin can call X related functions to perform all it wants, it just has no way to safely obtain errors from X11.

For more one could read https://www.remlab.net/op/xlib.shtml it has a pretty good explanation on that matter.

The thing with winit is that it already exposes the error via XConnection, however it's hidden from the end users and is being used internally by some crates that know about it.

So, I'd suggest to document that and expose last error from X11 in some more defined way. Either via proving it in some sort of Arc<Mutex<>> or via pointer to some function to call in hope to obtain the error information in a thread safe way.

Note, that XCB doesn't have mentioned issues in general, however GLX and some other APIs do require the use of Xlib unfortunately.

cc @maroider

[maroider]

My first instinct would be to reach for something like this:

fn register_my_err_handler() {
    static PREV_ERR_HANDLER: Mutex<Option<fn()>> = Mutex::new(None);
    extern "C" fn my_error_handler(display: *mut Display, error: *mut XErrorEvent) {
        if let Some(prev) = PREV_ERR_HANDLER.lock().unwrap() {
            unsafe { prev(display, error) }
        }
    }

    let prev = unsafe { XSetErrorHandler(Some(my_error_handler)) };
    PREV_ERR_HANDLER.lock().unwrap() = prev;
}

Is this sort of ad-hoc error handler chaining not advisable?

[kchibisov]

Is this sort of ad-hoc error handler chaining not advisable?

I'm not sure that it should work or I don't quite understand what are you supposed to do like that. The problem is that even if you use locks, we must ensure that winit is blocked before doing any xlib calls, and applications would block on the same mutex winit is blocked when they try to touch xlib.

The issue is that changing error handler could lead to breakage in multithreaded environment (read the link it's kind of explained there, since it's not obvious that the error handling is a per process global). So the only solution I can come up with, is that winit will provide a way to access error from xlib, but not to set error handler and advice from not setting error handler. How it could be exposed is a question. Right now we have an XConnection field like last_error, but it's a bit cumbersome to find it. We can provide an optional hook when building event loop so the users can pass a function pointer(not lambda) when building it and that will be used to deliver errors to the end user or winit.

However for wider integration this function should return an Xerror.

One more thing I can think of is if we provide a way to add a hook for winit's Xlib error handling. So we'd have a global array of functions that the winit error handling will call once received error(In a loop, passing the information from the X11 directly). Those functions will return an indicator bool, whether the error was handled or not. The issue is that how it should be registered. And for that purpose we'd need a function that could be passed to other applications that they can call it with their error handling function to register inside winit.
I'd probably suggest to put it in a static method on the event loop, so I could pass this method to the glutin display builder. I can't think of a better way to do this.

Such function could look like

impl EventLoop {
  fn register_xlib_error_hook(hook: fn(*mut Display, *mut error) -> bool );
}

Then in x11's event loop we'd have something like.

static ERROR_HOOKS: Mutex<Vec<fn(*mut Display, *mut error>>;

extern "C" fn winit_error_handler(display *mut Display, error: *mut XErrorEvent) {
  let hooks = HOOKS.lock();
  let mut should_warn = false;
  for hook in hooks {
      should_warn |= hook(display, error);
  }
  if should_warn {
      error!("X11 error {}", getErrorDescription);
  }  
}

The last solution is kind of Ok-ish, not very friendly, but Xlib is just a pain to begin with when it comes to error handling.

[maroider]

I suppose that would work. Can you PR that, then?