From a8e363aeed4919dc46490e6699a34d2e18b2170c Mon Sep 17 00:00:00 2001 From: SloppaElse Date: Wed, 30 Sep 2026 17:30:10 +0200 Subject: [PATCH 1/2] fix(gles): keep surface-created EGL context alive past Surface.Destroy On Linux the AdapterContext that backs an EGL context may be created by the Surface instead of the Instance (Wayland, or when the detected window kind differs from the real window's kind, e.g. an X11 window under a Wayland session). In that case Surface.SetFail owned the context and Surface.Destroy freed it. The Adapter, Device and Queue created from that Surface hold the same *AdapterContext and outlive the Surface. Closing the window freed the EGL context first; the later Device.Destroy locked a nil EGL context ("gles: AdapterContext.Lock: nil egl context") and dereferenced the nil *gl.Context in DeleteVertexArrays, crashing during app shutdown: panic: runtime error: invalid memory address or nil pointer dereference ... gles.(*Device).Destroy -> gl.(*Context).DeleteVertexArrays(0x0) Adopt ownership on the Instance when CreateSurface has to create the context. The Instance is released after the Device and Surface, so the shared context now lives as long as everything that uses it. Region Path B now always returns ownsContext=false. Adds an integration regression test that destroys a Surface sharing an adopted context, then destroys the Device, then the Instance. --- hal/gles/api_linux.go | 78 +++++++++++++++++++++++-------- hal/gles/lifetime_repro_test.go | 81 +++++++++++++++++++++++++++++++++ hal/gles/resource_linux.go | 9 ++-- 3 files changed, 146 insertions(+), 22 deletions(-) create mode 100644 hal/gles/lifetime_repro_test.go diff --git a/hal/gles/api_linux.go b/hal/gles/api_linux.go index 39d44d8..6cb055c 100644 --- a/hal/gles/api_linux.go +++ b/hal/gles/api_linux.go @@ -7,6 +7,7 @@ package gles import ( "fmt" + "sync" "github.com/gogpu/gputypes" "github.com/gogpu/wgpu/hal" @@ -83,13 +84,23 @@ func (Backend) CreateInstance(_ *hal.InstanceDescriptor) (hal.Instance, error) { } // Instance implements hal.Instance for the OpenGL backend on Linux. -// ctx is non-nil when an instance-level EGL context was created successfully -// (X11/headless). On Wayland it may be nil — CreateSurface provides a -// Surface-owned AdapterContext when a window handle is available. +// ctx is non-nil when an EGL context is available: either created at instance +// init (X11/headless) or adopted from the first CreateSurface on Wayland when +// no instance context could be created. The Instance owns ctx for its whole +// lifetime and destroys it LAST in Destroy. type Instance struct { + mu sync.Mutex ctx *AdapterContext } +// context returns the current instance AdapterContext. Safe to call +// concurrently with CreateSurface adopting the surface-created context. +func (i *Instance) context() *AdapterContext { + i.mu.Lock() + defer i.mu.Unlock() + return i.ctx +} + // CreateSurface creates an OpenGL surface from window handles. // On Linux: displayHandle and windowHandle are platform-specific. // For X11: displayHandle is X11 Display*, windowHandle is Window. @@ -118,17 +129,17 @@ func (i *Instance) CreateSurface(target hal.SurfaceTarget) (hal.Surface, error) // Path A: share Instance AdapterContext (X11 — context matches window system). // Do NOT share if Instance context is surfaceless (headless/Wayland fallback) // and Surface needs a window — the EGL display won't support eglCreateWindowSurface. - if i.ctx != nil && i.ctx.EGL() != nil && i.ctx.GL() != nil && i.ctx.EGL().WindowKind() == targetWindowKind { + if instCtx := i.context(); instCtx != nil && instCtx.EGL() != nil && instCtx.GL() != nil && instCtx.EGL().WindowKind() == targetWindowKind { hal.Logger().Info("gles: surface sharing Instance AdapterContext") - glCtx := i.ctx.Lock() + glCtx := instCtx.Lock() version := glCtx.GetString(gl.VERSION) renderer := glCtx.GetString(gl.RENDERER) - i.ctx.Unlock() + instCtx.Unlock() return &Surface{ displayHandle: displayHandle, windowHandle: windowHandle, - ctx: i.ctx, - eglDisplay: i.ctx.EGL().Display(), + ctx: instCtx, + eglDisplay: instCtx.EGL().Display(), ownsContext: false, version: version, renderer: renderer, @@ -166,20 +177,45 @@ func (i *Instance) CreateSurface(target hal.SurfaceTarget) (hal.Surface, error) version := glCtx.GetString(gl.VERSION) renderer := glCtx.GetString(gl.RENDERER) - hal.Logger().Info("gles: surface created with owned AdapterContext", + hal.Logger().Info("gles: surface created with AdapterContext", "version", version, "renderer", renderer, "gles", config.GLES) + // The GL context is shared with the Adapter/Device/Queue created from this + // Surface, and those outlive the Surface. Adopt ownership on the Instance + // (destroyed LAST) instead of the Surface, so Surface.Destroy cannot free a + // context the Device is still using. Otherwise closing the window frees the + // EGL context first and the later Device.Destroy() locks a nil EGL context + // and panics on shutdown. + adapterCtx := NewAdapterContext(eglCtx, glCtx, true) + if prev := i.adoptContext(adapterCtx); prev != nil { + prev.Destroy() + } + return &Surface{ displayHandle: displayHandle, windowHandle: windowHandle, - ctx: NewAdapterContext(eglCtx, glCtx, true), + ctx: adapterCtx, eglDisplay: eglCtx.Display(), - ownsContext: true, + ownsContext: false, version: version, renderer: renderer, }, nil } +// adoptContext makes ctx the instance AdapterContext and returns any previous +// context so the caller can destroy it. Ownership moves to the Instance so the +// context lifetime covers the Surface and the Device that share it. +func (i *Instance) adoptContext(ctx *AdapterContext) (prev *AdapterContext) { + i.mu.Lock() + defer i.mu.Unlock() + prev = i.ctx + i.ctx = ctx + if prev == ctx { + return nil + } + return prev +} + // EnumerateAdapters returns available OpenGL adapters. // Uses surface context (preferred), instance context (X11/headless), or placeholder. func (i *Instance) EnumerateAdapters(surfaceHint hal.Surface) []hal.ExposedAdapter { @@ -188,10 +224,11 @@ func (i *Instance) EnumerateAdapters(surfaceHint hal.Surface) []hal.ExposedAdapt return []hal.ExposedAdapter{surface.GetAdapterInfo()} } - // Priority 2: instance-level AdapterContext (created in CreateInstance via pbuffer/surfaceless) - if i.ctx != nil && i.ctx.GL() != nil { + // Priority 2: instance-level AdapterContext (created in CreateInstance via + // pbuffer/surfaceless, or adopted from the first CreateSurface on Wayland). + if instCtx := i.context(); instCtx != nil && instCtx.GL() != nil { return []hal.ExposedAdapter{ - makeAdapterFromContext(i.ctx), + makeAdapterFromContext(instCtx), } } @@ -255,10 +292,15 @@ func makeAdapterFromContext(ctx *AdapterContext) hal.ExposedAdapter { } } -// Destroy releases the instance resources. +// Destroy releases the instance resources. The Instance owns its AdapterContext +// (either created at init or adopted from a Surface) and is released after the +// Device and Surface, so this is the last destroy of the shared EGL context. func (i *Instance) Destroy() { - if i.ctx != nil { - i.ctx.Destroy() - i.ctx = nil + i.mu.Lock() + ctx := i.ctx + i.ctx = nil + i.mu.Unlock() + if ctx != nil { + ctx.Destroy() } } diff --git a/hal/gles/lifetime_repro_test.go b/hal/gles/lifetime_repro_test.go new file mode 100644 index 0000000..de98e64 --- /dev/null +++ b/hal/gles/lifetime_repro_test.go @@ -0,0 +1,81 @@ +//go:build integration && linux && !(js && wasm) + +package gles + +import ( + "runtime" + "testing" + + "github.com/gogpu/wgpu/hal/gles/egl" + "github.com/gogpu/wgpu/hal/gles/gl" +) + +// newTestAdapterContext creates a real EGL/GL context wrapped in an +// AdapterContext, or skips when no EGL display is available. +func newTestAdapterContext(t *testing.T) *AdapterContext { + t.Helper() + runtime.LockOSThread() + t.Cleanup(runtime.UnlockOSThread) + + if err := egl.Init(); err != nil { + t.Skipf("egl.Init() failed: %v", err) + } + config := egl.DefaultContextConfig() + config.GLES = false + eglCtx, err := egl.NewContext(config) + if err != nil { + t.Skipf("egl.NewContext() failed: %v", err) + } + if err := eglCtx.MakeCurrent(); err != nil { + eglCtx.Destroy() + t.Fatalf("MakeCurrent failed: %v", err) + } + glCtx := &gl.Context{} + if err := glCtx.Load(egl.GetGLProcAddress); err != nil { + eglCtx.Destroy() + t.Fatalf("GL load failed: %v", err) + } + _ = egl.MakeCurrent(eglCtx.Display(), egl.NoSurface, egl.NoSurface, egl.NoContext) + return NewAdapterContext(eglCtx, glCtx, true) +} + +// TestSurfaceDoesNotDestroyAdoptedContext is the regression test for the +// shutdown panic: when a Surface had to create its own EGL context (Wayland / +// window-kind mismatch), the context is now adopted by the Instance, so +// Surface.Destroy must NOT free it while the Device still uses it. +// +// Before the fix, Surface.Destroy called AdapterContext.Destroy (ownsContext +// was true), deleting the context that the Device held. A later Device.Destroy +// locked a nil EGL context and dereferenced nil *gl.Context -> SIGSEGV. +func TestSurfaceDoesNotDestroyAdoptedContext(t *testing.T) { + adapterCtx := newTestAdapterContext(t) + + inst := &Instance{} + if prev := inst.adoptContext(adapterCtx); prev != nil { + t.Fatalf("adoptContext returned unexpected previous context %p", prev) + } + + // Surface returned by CreateSurface Path B: shares the adopted context. + surf := &Surface{ctx: adapterCtx, ownsContext: false} + device := &Device{ctx: adapterCtx, vao: 1} + + // Window close destroys the Surface first. + surf.Destroy() + + // The shared context must still be alive for the Device. + if adapterCtx.eglCtx == nil || adapterCtx.gl == nil { + t.Fatal("Surface.Destroy freed the context the Device still uses") + } + + // Shutdown destroys the Device next — must not nil-deref. + device.Destroy() + if device.vao != 0 { + t.Fatalf("Device.Destroy left vao set: %d", device.vao) + } + + // Instance is released last and frees the context. + inst.Destroy() + if adapterCtx.eglCtx != nil || adapterCtx.gl != nil { + t.Fatal("Instance.Destroy did not release the adopted context") + } +} diff --git a/hal/gles/resource_linux.go b/hal/gles/resource_linux.go index 9099fa4..fd10fba 100644 --- a/hal/gles/resource_linux.go +++ b/hal/gles/resource_linux.go @@ -14,10 +14,11 @@ import ( ) // Surface implements hal.Surface for OpenGL on Linux. -// When Instance has a pre-created AdapterContext (X11/headless), ownsContext=false — -// Surface shares Instance's context (Windows AdapterContext parity). -// When Instance has no context (Wayland), ownsContext=true — Surface owns its own -// AdapterContext (intentional Wayland divergence). +// Surface never owns the AdapterContext: either it shares an instance context +// (X11/headless), or the context it creates for Wayland / window-kind mismatch +// is adopted by the Instance at creation time. This keeps the shared EGL +// context alive for the Adapter/Device/Queue, which outlive the Surface. +// ownsContext is kept for backwards compatibility and is always false here. type Surface struct { displayHandle uintptr windowHandle uintptr From 207d1bd7e0c476e9be8af8fe50d9e13f89f96c9f Mon Sep 17 00:00:00 2001 From: SloppaElse Date: Wed, 30 Sep 2026 17:33:13 +0200 Subject: [PATCH 2/2] docs(changelog): note GLES surface context lifetime fix --- CHANGELOG.md | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ab883ca..b7af853 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,19 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [Unreleased] + +### Fixed + +- **GLES Linux surface context lifetime** — a Surface that had to create its own + EGL context (Wayland, or when the detected window kind did not match the real + window) owned that context and freed it in `Surface.Destroy`. The Adapter, + Device and Queue share the same `*AdapterContext` and outlive the Surface, so + closing the window freed the EGL context first and the later `Device.Destroy` + locked a nil EGL context and dereferenced nil `*gl.Context` in + `DeleteVertexArrays`, panicking on shutdown. `CreateSurface` now adopts the + context on the Instance, which is released after the Device and Surface. + ## [0.34.5] - 2026-09-07 ### Fixed