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 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