Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Call XInitThreads first; XCloseDisplay cleanup #1291

Conversation

RonaldZielaznicki
Copy link

  • By selecting this checkbox, I agree to license my contributions to this project under the license(s) described in the LICENSE file, and I have the right to do so or have received permission to do so by an employer or client I am producing work for whom has this right.

Closes #1290

Context

yppy provided some excellent advise on discord:

XInitThreads should be the first xlib function called (though im not sure i would believe in xlib multithreading...)

And after I mentioned an error code on exit, they also advised:

error 3 is badwindow
probably something is using it after XDestroyWindow

The discussion after lead to the realization that platform.deinit is called before the GPU backend is released. This caused a race condition where the GPU backend might interact with the window before it is released.

Changes

  • change: libx11.XInitThreads is now called first when initializing X11.
  • remove: X11's custom errorHandler in favor of the default handler's more verbose description.
  • change: platform.deinit is now called after the GPU backend has been released.

Some additional detail

To show why I removed the custom X11 error handler.

Custom Error Message

err(mach): X11: error code 3

Default Error Message

X Error of failed request:  BadWindow (invalid Window parameter)
  Major opcode of failed request:  148 ()
  Minor opcode of failed request:  1
  Resource id in failed request:  0x5400002
  Serial number of failed request:  2035
  Current serial number in output stream:  2036

// Xlibx11.XInitThreads must be called first
//
// if not, 'Unknown sequence number while processing queue' errors occur.
_ = libx11.XInitThreads();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like us to remove this from the codebase ASAP, since we don't have any multi-threading yet this should be doing nothing :) so let's get it out and debug any issues that arise from removing it

@slimsag slimsag merged commit 825a676 into hexops:main Oct 25, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

core: X11's update sometimes fails while polling for events
2 participants