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.

Sign up for GitHub

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

Check duplicate native surfaces #155

Merged
merged 2 commits into from
Nov 18, 2024
Merged

Check duplicate native surfaces #155

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7d1d1aa
Fix a segfault in wlEglCreatePlatformWindowSurfaceHook
kbrenneman Nov 14, 2024
83ea2aa
Check for a duplicate wl_surface in wlEglCreatePlatformWindowSurfaceHook
kbrenneman Nov 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 25 additions & 7 deletions src/wayland-eglsurface.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2652,7 +2652,10 @@ EGLSurface wlEglCreatePlatformWindowSurfaceHook(EGLDisplay dpy,
WlEglDisplay *display = wlEglAcquireDisplay(dpy);
WlEglPlatformData *data = NULL;
WlEglSurface *surface = NULL;
WlEglSurface *existingSurf = NULL;
struct wl_egl_window *window = (struct wl_egl_window *)nativeWin;
struct wl_surface *wsurf = NULL;
long int wver = 0;
EGLBoolean res = EGL_FALSE;
EGLint err = EGL_SUCCESS;
EGLint surfType;
Expand Down Expand Up @@ -2683,6 +2686,23 @@ EGLSurface wlEglCreatePlatformWindowSurfaceHook(EGLDisplay dpy,
goto fail;
}

getWlEglWindowVersionAndSurface(window, &wver, &wsurf);
if (wsurf == NULL) {
err = EGL_BAD_ALLOC;
goto fail;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Here we store our wl_surface in wsurf so we don't prematurely set it, but doesn't this leak the surface if we jump to fail later on (like in line 2708? Before this change I think this would get cleaned up as part of wlEglDestroySurface but now this isn't recorded in surface until later?

I think we can go ahead and set surface->wlSurface here? Afaict that shouldn't affect anything between here and 2779 and properly handles the surface cleanup.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

getWlEglWindowVersionAndSurface doesn't allocate anything, it just figures out the correct offset for the wl_surface pointer in the wl_egl_surface struct.

That said, using the same surface variable in the loop would cause problems in the failure path, because it'll try to clean up an existing WlEglSurface.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah good point, thanks that looks better now.


// Make sure that we don't have any existing EGLSurfaces for this
// wl_surface. The driver_private check above isn't sufficient for this: If
// the app calls wl_egl_window_create more than once on the same
// wl_surface, then it would get multiple wl_egl_window structs.
wl_list_for_each(existingSurf, &display->wlEglSurfaceList, link) {
if (existingSurf->wlSurface == wsurf) {
err = EGL_BAD_ALLOC;
goto fail;
}
}

res = data->egl.getConfigAttrib(dpy, config, EGL_SURFACE_TYPE, &surfType);

if (!res || !(surfType & EGL_STREAM_BIT_KHR)) {
Expand Down Expand Up @@ -2757,9 +2777,8 @@ EGLSurface wlEglCreatePlatformWindowSurfaceHook(EGLDisplay dpy,
// Create per surface wayland queue
surface->wlEventQueue = wl_display_create_queue(display->nativeDpy);

getWlEglWindowVersionAndSurface(window,
&surface->wlEglWinVer,
&surface->wlSurface);
surface->wlEglWinVer = wver;
surface->wlSurface = wsurf;

err = assignWlEglSurfaceAttribs(surface, attribs);
if (err != EGL_SUCCESS) {
Expand Down Expand Up @@ -2843,15 +2862,14 @@ EGLSurface wlEglCreatePlatformWindowSurfaceHook(EGLDisplay dpy,
return surface;

fail:
if (surface->drmSyncobjHandle) {
drmSyncobjDestroy(display->drmFd, surface->drmSyncobjHandle);
}

if (drmSyncobjFd > 0) {
close(drmSyncobjFd);
}

if (surface) {
if (surface->drmSyncobjHandle) {
drmSyncobjDestroy(display->drmFd, surface->drmSyncobjHandle);
}
wlEglDestroySurface(display, surface);
}

Expand Down