Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion internal/driver/glfw/loop.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
5 changes: 3 additions & 2 deletions internal/driver/glfw/window.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down
13 changes: 13 additions & 0 deletions internal/driver/glfw/window_desktop.go
Original file line number Diff line number Diff line change
Expand Up @@ -892,6 +892,19 @@ func (w *window) view() *glfw.Window {
return w.viewport
}

// destroyViewport destroys the underlying GLFW window and clears the pointer

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can you make this more succinct?
It reads AI generated because it refers to behaviour in other methods as well.
The docs on a method should describe what it does only - not how others respond to it.
Also no need to mention it has the same threading semantics as the rest of the code in the same file.

// 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
Expand Down
14 changes: 14 additions & 0 deletions internal/driver/glfw/window_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")

Expand Down
13 changes: 13 additions & 0 deletions internal/driver/glfw/window_wasm.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading