Panic when using autogenerated quit in menu if some windows have not been shown

[imran-iq]

Checklist

• I have searched the issue tracker for open issues that relate to the same problem, before opening a new one.
• This issue only relates to a single bug. I will open new issues for any other problems.

Describe the bug

Hi 👋 . First off amazing library, I am having a lot of fun building a UI with it.

I am running into a bug regarding the menu (I think). If I run the code provided and quit the app from the main menu without showing the second window at least once. The program will panic/sigsegv on exit with the following:

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x99e754]

goroutine 12 [running]:
github.com/go-gl/glfw/v3.3/glfw.(*Window).SetShouldClose.func1(0x10000000045f8d6?)
        /home/iiqbal/go/pkg/mod/github.com/go-gl/glfw/v3.3/[email protected]/window.go:380 +0x14
github.com/go-gl/glfw/v3.3/glfw.(*Window).SetShouldClose(0x9eaa20?, 0x10?)
        /home/iiqbal/go/pkg/mod/github.com/go-gl/glfw/v3.3/[email protected]/window.go:380 +0x25
fyne.io/fyne/v2/internal/driver/glfw.(*window).closed(0xc00004a200?, 0x0?)
        /home/iiqbal/go/pkg/mod/fyne.io/fyne/[email protected]/internal/driver/glfw/window_desktop.go:320 +0x25
fyne.io/fyne/v2/internal/driver/glfw.addMissingQuit.func1()
        /home/iiqbal/go/pkg/mod/fyne.io/fyne/[email protected]/internal/driver/glfw/menu.go:35 +0x87
fyne.io/fyne/v2/widget.(*menuItem).trigger(0xc000538240)
        /home/iiqbal/go/pkg/mod/fyne.io/fyne/[email protected]/widget/menu_item.go:205 +0x3c
fyne.io/fyne/v2/widget.(*menuItem).Tapped(0xc00008bfb0?, 0xc0004cd801?)
        /home/iiqbal/go/pkg/mod/fyne.io/fyne/[email protected]/widget/menu_item.go:151 +0x58
fyne.io/fyne/v2/internal/driver/glfw.(*window).mouseClickedHandleTapDoubleTap.func1()
        /home/iiqbal/go/pkg/mod/fyne.io/fyne/[email protected]/internal/driver/glfw/window.go:634 +0x26
fyne.io/fyne/v2/internal/driver/common.(*Window).RunEventQueue(0x0?)
        /home/iiqbal/go/pkg/mod/fyne.io/fyne/[email protected]/internal/driver/common/window.go:35 +0x3e
created by fyne.io/fyne/v2/internal/driver/glfw.(*gLDriver).createWindow.func1
        /home/iiqbal/go/pkg/mod/fyne.io/fyne/[email protected]/internal/driver/glfw/window.go:946 +0x136
exit status 2

How ever if I close the app by hitting the X in the corner, no crash happens

How to reproduce

1. Run the program with go run .
2. Do not show the second window
3. Quit the app from the main menu

Screenshots

No response

Example code

package main

import (
	"fyne.io/fyne/v2"
	"fyne.io/fyne/v2/app"
	"fyne.io/fyne/v2/widget"
)

func main() {
	a := app.NewWithID("test")

	w2 := a.NewWindow("Hello again")
	t2 := widget.NewLabel("Hello from other window")
	w2.SetContent(t2)
	w2.SetCloseIntercept(func() {
		w2.Hide()
	})

	w1 := a.NewWindow("Hello")
	t1 := widget.NewLabel("Hello from the first window!")
	s := fyne.NewMenuItem(
		"Show",
		func() { w2.Show() },
	)
	m := fyne.NewMenu("File", s)
	w1.SetMainMenu(fyne.NewMainMenu(m))

	w1.SetMaster()

	w1.SetContent(t1)
	w1.Resize(fyne.NewSize(200, 200))
	w1.ShowAndRun()
}

Fyne version

2.3.4

Go compiler version

1.19.8

Operating system and version

Debian 12

Additional Information

If I change the code to remove the menu (and just a have a show button for w2) I am unable to recreate the crash

[imran-iq]

I /think/ I found the source:

We may need a nil check here:

fyne/internal/driver/glfw/window_desktop.go

Line 320 in 6558290

viewport.SetShouldClose(false) // reset the closed flag until we check the veto in processClosed

Called from here:

fyne/internal/driver/glfw/menu.go

Line 35 in 6558290

glWin.closed(glWin.view())

This is called on w2 its viewport is nil (presumably because it has never been shown) at least from running this in delve

Frame 6: /home/iiqbal/go/pkg/mod/fyne.io/fyne/[email protected]/internal/driver/glfw/window_desktop.go:320 (PC: d8ab08)
   315: func (w *window) refresh(_ *glfw.Window) {
   316:         w.processRefresh()
   317: }
   318:
   319: func (w *window) closed(viewport *glfw.Window) {
=> 320:         viewport.SetShouldClose(false) // reset the closed flag until we check the veto in processClosed
   321:
   322:         w.processClosed()
   323: }
   324:
   325: func fyneToNativeCursor(cursor desktop.Cursor) (*glfw.Cursor, bool) {
(dlv) p w
*fyne.io/fyne/v2/internal/driver/glfw.window {
        Window: fyne.io/fyne/v2/internal/driver/common.Window {
                eventQueue: *(*"fyne.io/fyne/v2/internal/async.UnboundedFuncChan")(0xc0000b4cf0),},
        viewport: *github.com/go-gl/glfw/v3.3/glfw.Window nil,
        <snipped>
        title: "Hello again",
        <snipped>
(dlv) p viewport
*github.com/go-gl/glfw/v3.3/glfw.Window nil

[imran-iq]

That looks like it is the case I made the following change

diff --git a/internal/driver/glfw/window_desktop.go b/internal/driver/glfw/window_desktop.go
index 4be8d1ab2..1d21f8be2 100644
--- a/internal/driver/glfw/window_desktop.go
+++ b/internal/driver/glfw/window_desktop.go
@@ -317,7 +317,9 @@ func (w *window) refresh(_ *glfw.Window) {
 }
 
 func (w *window) closed(viewport *glfw.Window) {
-       viewport.SetShouldClose(false) // reset the closed flag until we check the veto in processClosed
+       if viewport != nil {
+               viewport.SetShouldClose(false) // reset the closed flag until we check the veto in processClosed
+       }
 
        w.processClosed()
 }

and updated my go.mod file to point to the modified source and the process no longer panics on exit.

Let me know if you would like me to open up a pr with the fix, or if there is a better way of doing this (maybe checking in the menu.go file?)

[Jacalz]

Thanks for a well-written bug report and some very useful debugging. Feel free to open a PR against the develop branch :)

[andydotxyz]

Thanks so much for your help, this is now landed on develop :)