Completion entry causes crashing

[syllith]

I have been experiencing a bug with Fyne-X regarding the completion widget. I am utilizing Fynes app tab to display a completion entry. It seems once text is entered in the completion widget, then going to another tab for a few minutes (presumably for how long it takes Go's garbage collection to clean up), then switching back to the tab with the completion entry in it, it causes my application to panic with this stack trace:

panic: runtime error: invalid memory address or nil pointer dereference
[signal 0xc0000005 code=0x0 addr=0xa8 pc=0x7ff67f1f08c0]

goroutine 20 [running, locked to thread]:
fyne.io/x/fyne/widget.(*CompletionEntry).maxSize(0xc00161c900)
        C:/Users/kinlougn/go/pkg/mod/fyne.io/x/[email protected]/widget/completionentry.go:98 +0

Upon going to this section of the completionentry.go file, it seems I can resolve the issue by adding this right after we populate the cnv variable:

if cnv == nil {
    return fyne.NewSize(0, 0)
}

In my opinion, it seems like Go's garbage collection is cleaning up something it shouldn't, and when I switch back to the app tab with the completion entry, it tries to reference it but it's no longer there, causing the application to panic.

[andydotxyz]

Perhaps the widget is assuming the renderer will always be the same? We support cleaning up renderers when not needed - it is part of the widget contract...

[syllith]

Sorry for the (very) late response. I'm not gonna lie, I'm not entirely sure what you mean by this and was hoping you could help clarify. I'm going to sound pretty ignorant, but please know it's not intentional. You say you support cleaning up renderers (not even sure what a renderer is) when not needed, but in this case, its still needed, its just hidden because we're in another app tab. By widget contract, I assume that this is the base behavior you expect the widgets to have? I'm sorry, but I'm a bit clueless on the inner workings of Fyne.

It seems, from my limited knowledge, that the maxSize function isn't accounting for arguments that could be nil. Would it not be better to be safe and include a check for nil to prevent this from happening?

[andydotxyz]

In my opinion, it seems like Go's garbage collection is cleaning up something it shouldn't, and when I switch back to the app tab with the completion entry, it tries to reference it but it's no longer there, causing the application to panic.

What I mean is that the renderer for a widget can be nil - we may clean up the render state of a widget that is invisible - and then re-create it when it will be shown again.

It seems, from my limited knowledge, that the maxSize function isn't accounting for arguments that could be nil. Would it not be better to be safe and include a check for nil to prevent this from happening?

That may be the case, I was just trying to explain where the nil may be coming from instead of the suspicion that it was a Go GC bug.

[syllith]

Hey, so I was wondering if you'd consider adding this nil check to the code. It's been months and I've been hoping that the issue would have been resolved by now, but I need to go in and re-patch this every time fyne-x gets updated. If this seems like a safe fix to you, could you consider adding it? I've been using it for months and I've yet to see a negative side effect from it. Would you consider adding this nil check to the official repo?