diff --git a/internal/driver/glfw/loop.go b/internal/driver/glfw/loop.go index 814f09f55b..7d1d273afc 100644 --- a/internal/driver/glfw/loop.go +++ b/internal/driver/glfw/loop.go @@ -193,7 +193,7 @@ func (d *gLDriver) runGL() { func (d *gLDriver) destroyWindow(w *window, index int) { w.visible = false - w.viewport.Destroy() + w.destroyViewport() w.destroy(d) if index < len(d.windows)-1 { diff --git a/internal/driver/glfw/window.go b/internal/driver/glfw/window.go index d53832fbc4..6d154595f8 100644 --- a/internal/driver/glfw/window.go +++ b/internal/driver/glfw/window.go @@ -896,10 +896,11 @@ func (w *window) triggerMainMenuShortcut(sh fyne.Shortcut) bool { } func (w *window) RunWithContext(f func()) { - if w.isClosing() { + v := w.view() + if v == nil { return } - w.view().MakeContextCurrent() + v.MakeContextCurrent() f() diff --git a/internal/driver/glfw/window_desktop.go b/internal/driver/glfw/window_desktop.go index 854c596f77..ee8f743787 100644 --- a/internal/driver/glfw/window_desktop.go +++ b/internal/driver/glfw/window_desktop.go @@ -892,6 +892,19 @@ func (w *window) view() *glfw.Window { return w.viewport } +// destroyViewport destroys the underlying GLFW window and clears the pointer +// first, so a later view() / isClosing() observes the destruction and never +// hands out a pointer to an already-destroyed window. Safe to call once; later +// calls are no-ops. Must run on the main goroutine, like the rest of the GLFW +// lifecycle. +func (w *window) destroyViewport() { + vp := w.viewport + w.viewport = nil + if vp != nil { + vp.Destroy() + } +} + // wrapInnerWindow is a no-op to match what the web driver provides func wrapInnerWindow(*container.InnerWindow, fyne.Window, *gLDriver) fyne.Window { return nil diff --git a/internal/driver/glfw/window_test.go b/internal/driver/glfw/window_test.go index 92bb7d4fd4..bfedc31cc7 100644 --- a/internal/driver/glfw/window_test.go +++ b/internal/driver/glfw/window_test.go @@ -1818,6 +1818,20 @@ func TestWindow_ClosedBeforeShow(t *testing.T) { assert.NotPanics(t, func() { w.closed(nil) }) } +// fyne#3874: viewport.Destroy() does not nil w.viewport, so view() keeps +// handing a dead handle to the draw thread. After Destroy(), view() must +// return nil — otherwise RunWithContext drives MakeContextCurrent on a +// destroyed GLFW window (SIGSEGV on macOS, BadWindow on X11). +func TestWindow_RunWithContext_AfterViewportDestroyed(t *testing.T) { + w := createWindow("Race-Destroyed") + + runOnMain(func() { + w.window.destroyViewport() + }) + + require.Nil(t, w.window.view(), "view() must be nil after destroyViewport()") +} + func TestWindow_SetContent_Twice(t *testing.T) { w := createWindow("Test") diff --git a/internal/driver/glfw/window_wasm.go b/internal/driver/glfw/window_wasm.go index ec24415990..b5bc53d88d 100644 --- a/internal/driver/glfw/window_wasm.go +++ b/internal/driver/glfw/window_wasm.go @@ -592,6 +592,19 @@ func (w *window) view() *glfw.Window { return w.viewport } +// destroyViewport destroys the underlying GLFW window and clears the pointer +// first, so a later view() / isClosing() observes the destruction and never +// hands out a pointer to an already-destroyed window. Safe to call once; later +// calls are no-ops. Must run on the main goroutine, like the rest of the GLFW +// lifecycle. +func (w *window) destroyViewport() { + vp := w.viewport + w.viewport = nil + if vp != nil { + vp.Destroy() + } +} + // wrapInner represents a window that is provided by an InnerWindow container in the canvas. type wrapInner struct { fyne.Window