Do not consider the EGLDisplay owned when creating from a pre-existing X11 Display

[Melchizedek6809]

When creating an EGLDisplay from an X11 Display, the driver often returns the same EGLDisplay for each Driver. This means that we cannot call eglTerminate on the resulting display and shouldn't consider it as an owned display. Terminating this display may kill other live connections. This fixes a panic when closing a second window on X11 (servo/servo#48075).

[mrobinson]

An EGL display is created unconditionally above, so how is this not leaking the EGL display? I think there's a deeper problem here.

[Melchizedek6809]

That's what I initially thought as well, but reading the spec for eglGetPlatformDisplay it has the following section:

Multiple calls made to eglGetPlatformDisplay with the same parameters will return the same EGLDisplay handle.

This means we get the same EGLDisplay since we're using the same X11 Window, the second eglInitialize call is treated as a noop, but once we drop a Connection and call eglTerminate this also marks resources from other contexts sharing the same Connection for release, leading to the panics.

But yeah, there might be a better solution, still struggling to wrap my head around the whole connection/context handling.

https://registry.khronos.org/EGL/sdk/docs/man/html/eglInitialize.xhtml
https://registry.khronos.org/EGL/sdk/docs/man/html/eglTerminate.xhtml
https://registry.khronos.org/EGL/sdk/docs/man/html/eglGetPlatformDisplay.xhtml

[Melchizedek6809]

I've looked some more into this issue, and think I'm starting to understand this issue.

The EGL spec states that eglGetPlatformDisplay will return the same handle if given the same parameters.

The Xlib manual also does not specify that a call to XOpenDisplay returns a unique Display*, just that it returns a Display* for the given display_name, and in my testing multiple calls to it return the same pointer.

So we now have the problem that depending on how the Connections were created we have multiple Connections with the is_owner flag set that actually share pointers. If we now Drop a Connection (as happens if we close a Servo window) we call eglTerminate which also invalidates the Connection of the other existing window.

Still not sure how to best address this, seems we need some sort of RefCounting scheme to keep track of which Display/EGLDisplay instances are used and only call eglTerminate/XCloseDisplay once we drop the final Ref.

https://tronche.com/gui/x/xlib/display/opening.html

[Melchizedek6809]

@mrobinson
Alright, just turned the assert on the eglTerminate return value into a debug assert.

[mrobinson]

@Melchizedek6809 Thank you for the fix!

[Melchizedek6809]

@Melchizedek6809 Thank you for the fix!

Glad to have helped :)