Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Completion entry causes crashing #76

Open
syllith opened this issue Oct 9, 2023 · 4 comments
Open

Completion entry causes crashing #76

syllith opened this issue Oct 9, 2023 · 4 comments

Comments

@syllith
Copy link

syllith commented Oct 9, 2023

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
Copy link
Member

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
Copy link
Author

syllith commented Dec 4, 2023

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
Copy link
Member

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
Copy link
Author

syllith commented Apr 29, 2024

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants