From c7cc4876bf5784ddae2151ae15c0c3c84f0b5bad Mon Sep 17 00:00:00 2001 From: Dmitry Zolotarev Date: Mon, 18 May 2026 14:55:59 +0300 Subject: [PATCH 1/3] test: repro RunWithContext race and dead viewport (#3874) --- internal/driver/glfw/window_test.go | 54 +++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/internal/driver/glfw/window_test.go b/internal/driver/glfw/window_test.go index 92bb7d4fd4..a7d6b04aaa 100644 --- a/internal/driver/glfw/window_test.go +++ b/internal/driver/glfw/window_test.go @@ -7,6 +7,8 @@ import ( "net/url" "os" "runtime" + "sync" + "sync/atomic" "testing" "time" @@ -1818,6 +1820,58 @@ func TestWindow_ClosedBeforeShow(t *testing.T) { assert.NotPanics(t, func() { w.closed(nil) }) } +// fyne-io/fyne#3874: w.closing / w.viewport in RunWithContext are read +// without synchronization, and viewport.Destroy() does not nil the pointer. +// Two failure modes: data race on w.closing, and MakeContextCurrent on a +// destroyed viewport. Run with `-race`. +func TestWindow_RunWithContext_DataRace(t *testing.T) { + w := createWindow("Race3874-DataRace") + defer w.Close() + + win := w.window + + var ( + stop int32 + wg sync.WaitGroup + ) + + wg.Add(1) + go func() { + defer wg.Done() + for atomic.LoadInt32(&stop) == 0 { + _ = win.isClosing() + _ = win.view() + runtime.Gosched() + } + }() + + for i := 0; i < 200; i++ { + runOnMain(func() { + win.closing = !win.closing + }) + } + runOnMain(func() { + win.closing = false + }) + + atomic.StoreInt32(&stop, 1) + wg.Wait() +} + +// 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("Race3874-Destroyed") + + runOnMain(func() { + w.window.viewport.Destroy() + }) + + require.Nil(t, w.window.view(), "view() must be nil after viewport.Destroy()") +} + func TestWindow_SetContent_Twice(t *testing.T) { w := createWindow("Test") From 96c11dc276436f5534766effb432f63e6fa230a3 Mon Sep 17 00:00:00 2001 From: Dmitry Zolotarev Date: Mon, 18 May 2026 15:18:41 +0300 Subject: [PATCH 2/3] fix: synchronize close state in glfw window (#3874) --- internal/driver/glfw/loop.go | 2 +- internal/driver/glfw/window.go | 9 +++++++-- internal/driver/glfw/window_desktop.go | 18 ++++++++++++++++++ internal/driver/glfw/window_test.go | 22 ++++++++-------------- 4 files changed, 34 insertions(+), 17 deletions(-) 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..168655ea3e 100644 --- a/internal/driver/glfw/window.go +++ b/internal/driver/glfw/window.go @@ -209,7 +209,9 @@ func (w *window) Close() { // Clean up accessibility resources w.cleanupAccessibilityForWindow() + w.closeLock.Lock() w.closing = true + w.closeLock.Unlock() w.viewport.SetShouldClose(true) cache.RangeTexturesFor(w.canvas, w.canvas.Painter().Free) @@ -896,10 +898,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() @@ -997,6 +1000,8 @@ func (w *window) doShowAgain() { } func (w *window) isClosing() bool { + w.closeLock.RLock() + defer w.closeLock.RUnlock() return w.closing || w.viewport == nil } diff --git a/internal/driver/glfw/window_desktop.go b/internal/driver/glfw/window_desktop.go index 854c596f77..baf80854d1 100644 --- a/internal/driver/glfw/window_desktop.go +++ b/internal/driver/glfw/window_desktop.go @@ -10,6 +10,7 @@ import ( "os" "runtime" "strings" + "sync" "time" "fyne.io/fyne/v2" @@ -75,6 +76,8 @@ type window struct { closing bool fixedSize bool + closeLock sync.RWMutex // guards closing and viewport + cursor desktop.Cursor customCursor *glfw.Cursor canvas *glCanvas @@ -886,12 +889,27 @@ func (w *window) create() { } func (w *window) view() *glfw.Window { + w.closeLock.RLock() + defer w.closeLock.RUnlock() if w.closing { return nil } return w.viewport } +// destroyViewport destroys the underlying GLFW window and clears the pointer +// under closeLock so subsequent view() / isClosing() calls observe the +// destruction atomically. Safe to call once; later calls are no-ops. +func (w *window) destroyViewport() { + w.closeLock.Lock() + vp := w.viewport + w.viewport = nil + w.closeLock.Unlock() + 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 a7d6b04aaa..6ad46aee18 100644 --- a/internal/driver/glfw/window_test.go +++ b/internal/driver/glfw/window_test.go @@ -1825,9 +1825,7 @@ func TestWindow_ClosedBeforeShow(t *testing.T) { // Two failure modes: data race on w.closing, and MakeContextCurrent on a // destroyed viewport. Run with `-race`. func TestWindow_RunWithContext_DataRace(t *testing.T) { - w := createWindow("Race3874-DataRace") - defer w.Close() - + w := createWindow("Race-DataRace") win := w.window var ( @@ -1845,14 +1843,10 @@ func TestWindow_RunWithContext_DataRace(t *testing.T) { } }() - for i := 0; i < 200; i++ { - runOnMain(func() { - win.closing = !win.closing - }) - } - runOnMain(func() { - win.closing = false - }) + // Production write path: Close() flips w.closing on the main thread. + // Without synchronization the race detector flags the read above. + w.Close() + time.Sleep(50 * time.Millisecond) atomic.StoreInt32(&stop, 1) wg.Wait() @@ -1863,13 +1857,13 @@ func TestWindow_RunWithContext_DataRace(t *testing.T) { // 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("Race3874-Destroyed") + w := createWindow("Race-Destroyed") runOnMain(func() { - w.window.viewport.Destroy() + w.window.destroyViewport() }) - require.Nil(t, w.window.view(), "view() must be nil after viewport.Destroy()") + require.Nil(t, w.window.view(), "view() must be nil after destroyViewport()") } func TestWindow_SetContent_Twice(t *testing.T) { From b3b6ddef5c188e43600c2444b6b1b8b014eb2d7f Mon Sep 17 00:00:00 2001 From: dmitry Date: Sat, 13 Jun 2026 01:56:07 +0300 Subject: [PATCH 3/3] fix: clear viewport pointer after Destroy to avoid use-after-free (#3874) --- internal/driver/glfw/window.go | 4 --- internal/driver/glfw/window_desktop.go | 13 +++------- internal/driver/glfw/window_test.go | 34 -------------------------- internal/driver/glfw/window_wasm.go | 13 ++++++++++ 4 files changed, 17 insertions(+), 47 deletions(-) diff --git a/internal/driver/glfw/window.go b/internal/driver/glfw/window.go index 168655ea3e..6d154595f8 100644 --- a/internal/driver/glfw/window.go +++ b/internal/driver/glfw/window.go @@ -209,9 +209,7 @@ func (w *window) Close() { // Clean up accessibility resources w.cleanupAccessibilityForWindow() - w.closeLock.Lock() w.closing = true - w.closeLock.Unlock() w.viewport.SetShouldClose(true) cache.RangeTexturesFor(w.canvas, w.canvas.Painter().Free) @@ -1000,8 +998,6 @@ func (w *window) doShowAgain() { } func (w *window) isClosing() bool { - w.closeLock.RLock() - defer w.closeLock.RUnlock() return w.closing || w.viewport == nil } diff --git a/internal/driver/glfw/window_desktop.go b/internal/driver/glfw/window_desktop.go index baf80854d1..ee8f743787 100644 --- a/internal/driver/glfw/window_desktop.go +++ b/internal/driver/glfw/window_desktop.go @@ -10,7 +10,6 @@ import ( "os" "runtime" "strings" - "sync" "time" "fyne.io/fyne/v2" @@ -76,8 +75,6 @@ type window struct { closing bool fixedSize bool - closeLock sync.RWMutex // guards closing and viewport - cursor desktop.Cursor customCursor *glfw.Cursor canvas *glCanvas @@ -889,8 +886,6 @@ func (w *window) create() { } func (w *window) view() *glfw.Window { - w.closeLock.RLock() - defer w.closeLock.RUnlock() if w.closing { return nil } @@ -898,13 +893,13 @@ func (w *window) view() *glfw.Window { } // destroyViewport destroys the underlying GLFW window and clears the pointer -// under closeLock so subsequent view() / isClosing() calls observe the -// destruction atomically. Safe to call once; later calls are no-ops. +// 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() { - w.closeLock.Lock() vp := w.viewport w.viewport = nil - w.closeLock.Unlock() if vp != nil { vp.Destroy() } diff --git a/internal/driver/glfw/window_test.go b/internal/driver/glfw/window_test.go index 6ad46aee18..bfedc31cc7 100644 --- a/internal/driver/glfw/window_test.go +++ b/internal/driver/glfw/window_test.go @@ -7,8 +7,6 @@ import ( "net/url" "os" "runtime" - "sync" - "sync/atomic" "testing" "time" @@ -1820,38 +1818,6 @@ func TestWindow_ClosedBeforeShow(t *testing.T) { assert.NotPanics(t, func() { w.closed(nil) }) } -// fyne-io/fyne#3874: w.closing / w.viewport in RunWithContext are read -// without synchronization, and viewport.Destroy() does not nil the pointer. -// Two failure modes: data race on w.closing, and MakeContextCurrent on a -// destroyed viewport. Run with `-race`. -func TestWindow_RunWithContext_DataRace(t *testing.T) { - w := createWindow("Race-DataRace") - win := w.window - - var ( - stop int32 - wg sync.WaitGroup - ) - - wg.Add(1) - go func() { - defer wg.Done() - for atomic.LoadInt32(&stop) == 0 { - _ = win.isClosing() - _ = win.view() - runtime.Gosched() - } - }() - - // Production write path: Close() flips w.closing on the main thread. - // Without synchronization the race detector flags the read above. - w.Close() - time.Sleep(50 * time.Millisecond) - - atomic.StoreInt32(&stop, 1) - wg.Wait() -} - // 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 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