Data binding with widget.List sometimes crash while scrolling

[fpabl0]

Describe the bug:

Using data binding for a widget.List sometimes crash while scrolling. It seems to be a race condition. The error is:

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

goroutine 8 [running]:
fyne.io/fyne/v2/data/binding.(*stringFromFloat).Get(0x0, 0xc0002be508, 0xc000380290, 0xc0002be5b8, 0xc00033c800)
        .../fyne/data/binding/convert.go:109 +0x26
fyne.io/fyne/v2/widget.(*Label).Bind.func1()
        .../fyne/widget/label.go:59 +0x3c
fyne.io/fyne/v2/data/binding.(*listener).DataChanged(0xc001616728)
        .../fyne/data/binding/binding.go:55 +0x28
fyne.io/fyne/v2/data/binding.processItems()
        .../fyne/data/binding/queue.go:56 +0x94
created by fyne.io/fyne/v2/data/binding.init.0
        .../fyne/data/binding/queue.go:15 +0x35
exit status 2

To Reproduce:

Steps to reproduce the behaviour:

1. Run fyne_demo
2. Click on the binding tutorial.
3. Try appending items and scrolling (sometimes it crashes)

Device (please complete the following information):

• OS: MacOS
• Version: 10.15.7 Catalina
• Go version: 1.16.2
• Fyne version: develop @ 23e0ed1

[andydotxyz]

Are you able to see where the nil creeps in? It looks rather strange - the trace implies that the data-bind version of UpdateItem is called with a nil DataItem but that should not be possible...

[fpabl0]

I will try to investigate

[fpabl0]

After some tests, the nil exception occurs because l.textSource is nil inside a the listener callback. I added this simple print:

l.textListener = binding.NewDataListener(func() {
     fmt.Printf("label: %p, data: %p\n", l, l.textSource)
     ...
})

And I got this:

label: 0xc001878a20, data: %!p(<nil>)
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x20 pc=0x42b3dc9]

goroutine 8 [running]:
fyne.io/fyne/v2/widget.(*Label).Bind.func1()
        /Users/pablofuentes/Desktop/fpabl0_git/forks/fyne/widget/label.go:62 +0x149
fyne.io/fyne/v2/data/binding.(*listener).DataChanged(0xc00485acb8)
        /Users/pablofuentes/Desktop/fpabl0_git/forks/fyne/data/binding/binding.go:55 +0x28
fyne.io/fyne/v2/data/binding.processItems()
        /Users/pablofuentes/Desktop/fpabl0_git/forks/fyne/data/binding/queue.go:56 +0x94
created by fyne.io/fyne/v2/data/binding.init.0
        /Users/pablofuentes/Desktop/fpabl0_git/forks/fyne/data/binding/queue.go:15 +0x35
exit status 2

My thought is that this occurs because there are on-going callbacks processing (that depends on textSource) when label.Unbind() is called (converting it to nil). Maybe there should be a way to cancel on-going callbacks when a binding is about to be destroyed. A Destroy method for bindings could be useful to fix this error.

[fpabl0]

This code reproduce the problem very quickly:

package main

import (
	"fmt"
	"time"

	"fyne.io/fyne/v2"
	"fyne.io/fyne/v2/app"
	"fyne.io/fyne/v2/data/binding"
	"fyne.io/fyne/v2/widget"
)

func main() {
	a := app.New()
	w := a.NewWindow("Test")
	w.Resize(fyne.NewSize(300, 300))

	bindstr := binding.NewString()

	label := widget.NewLabel("")

	label.Bind(bindstr)

	go func() {
		i := 0
		for {
			bindstr.Set(fmt.Sprintf("Counter: %d", i))
			i++
		}
	}()

	go func() {
		time.Sleep(1 * time.Second)
		label.Unbind()
	}()

	w.SetContent(label)

	w.ShowAndRun()
}

[fpabl0]

Maybe DataListener could have a new method called Destroy:

type DataListener interface {
	DataChanged()
        Destroy()
}

type listener struct {
	callback     func()
        destroyed  bool
}

func (l *listener) DataChanged() {
        if l.destroyed {
              return
        }
	l.callback()
}

func (l *listener) Destroy() {
       l.destroyed = true
}

And so DataItem:

type DataItem interface {
	AddListener(DataListener)
        Destroy()
	RemoveListener(DataListener)
}

binding.base should then implement it as:

func (b *base) Destroy() {
        b.lock.Lock()
        defer b.lock.Unlock()
	for _, listen := range b.listeners {
		listen.Destroy()
	}
}

Then in Label.Unbind():

func (l *Label) Unbind() {
	if l.textSource == nil || l.textListener == nil {
		return
	}

	l.textSource.RemoveListener(l.textListener)
        l.textSource.Destroy()
	l.textListener = nil
	l.textSource = nil
}

[andydotxyz]

This is on develop and also release/v2.0.x ready for v2.0.3 release

[vboehr]

To avoid this issue can we expose something like IsScrolling() to test if the scrolling is on before setting the data?

[Jacalz]

To avoid this issue can we expose something like IsScrolling() to test if the scrolling is on before setting the data?

I’m not quite sure if that’s relevant to this issue? This bug was resolved and closed around a year ago. If it is a request for a new feature, I think you might want to open a feature request instead.

[vboehr]

I am sorry and thank you for the comment provided.