Merge pull request #5361 from andydotxyz/fix/mergethreads

Make data binding thread safe
fyne-io · Jan 6, 2025 · d47fdda · d47fdda
2 parents ef1351e + 60c9002
commit d47fdda
Show file tree

Hide file tree

Showing 33 changed files with 1,006 additions and 632 deletions.
diff --git a/data/binding/binding.go b/data/binding/binding.go
@@ -26,7 +26,7 @@ var (
 type DataItem interface {
 	// AddListener attaches a new change listener to this DataItem.
 	// Listeners are called each time the data inside this DataItem changes.
-	// Additionally the listener will be triggered upon successful connection to get the current value.
+	// Additionally, the listener will be triggered upon successful connection to get the current value.
 	AddListener(DataListener)
 	// RemoveListener will detach the specified change listener from the DataItem.
 	// Disconnected listener will no longer be triggered when changes occur.
@@ -74,10 +74,17 @@ func (b *base) RemoveListener(l DataListener) {
 }
 
 func (b *base) trigger() {
+	var listeners []DataListener
 	b.listeners.Range(func(key, _ any) bool {
-		queueItem(key.(DataListener).DataChanged)
+		listeners = append(listeners, key.(DataListener))
 		return true
 	})
+
+	queueItem(func() {
+		for _, listen := range listeners {
+			listen.DataChanged()
+		}
+	})
 }
 
 // Untyped supports binding a any value.

diff --git a/data/binding/binding_exted_test.go b/data/binding/binding_exted_test.go
@@ -18,20 +18,17 @@ func TestBindUserType(t *testing.T) {
 		called = true
 	})
 	u.AddListener(fn)
-	waitForItems()
 	assert.True(t, called)
 
 	called = false
 	err = u.Set(user{name: "Replace"})
 	assert.Nil(t, err)
-	waitForItems()
 	assert.Equal(t, "User: Replace", val.String())
 	assert.True(t, called)
 
 	called = false
 	val = user{name: "Direct"}
 	_ = u.Reload()
-	waitForItems()
 	assert.True(t, called)
 	v, err = u.Get()
 	assert.Nil(t, err)
@@ -40,7 +37,6 @@ func TestBindUserType(t *testing.T) {
 	called = false
 	val.name = "FieldSet"
 	_ = u.Reload()
-	waitForItems()
 	assert.True(t, called)
 	v, err = u.Get()
 	assert.Nil(t, err)

diff --git a/data/binding/binding_test.go b/data/binding/binding_test.go
@@ -29,8 +29,6 @@ func TestBase_AddListener(t *testing.T) {
 	})
 	data.AddListener(fn)
 	assert.Equal(t, 1, syncMapLen(&data.listeners))
-
-	waitForItems()
 	assert.True(t, called)
 }
 
@@ -46,7 +44,6 @@ func TestBase_RemoveListener(t *testing.T) {
 	data.RemoveListener(fn)
 	assert.Equal(t, 0, syncMapLen(&data.listeners))
 
-	waitForItems()
 	data.trigger()
 	assert.False(t, called)
 }
@@ -57,7 +54,6 @@ func TestNewDataItemListener(t *testing.T) {
 		called = true
 	})
 
-	waitForItems()
 	fn.DataChanged()
 	assert.True(t, called)
 }

diff --git a/data/binding/binditems.go b/data/binding/binditems.go
@@ -67,11 +67,12 @@ func (b *boundBool) Get() (bool, error) {
 
 func (b *boundBool) Set(val bool) error {
 	b.lock.Lock()
-	defer b.lock.Unlock()
 	if *b.val == val {
+		b.lock.Unlock()
 		return nil
 	}
 	*b.val = val
+	b.lock.Unlock()
 
 	b.trigger()
 	return nil
@@ -85,12 +86,13 @@ type boundExternalBool struct {
 
 func (b *boundExternalBool) Set(val bool) error {
 	b.lock.Lock()
-	defer b.lock.Unlock()
 	if b.old == val {
+		b.lock.Unlock()
 		return nil
 	}
 	*b.val = val
 	b.old = val
+	b.lock.Unlock()
 
 	b.trigger()
 	return nil
@@ -158,11 +160,12 @@ func (b *boundBytes) Get() ([]byte, error) {
 
 func (b *boundBytes) Set(val []byte) error {
 	b.lock.Lock()
-	defer b.lock.Unlock()
 	if bytes.Equal(*b.val, val) {
+		b.lock.Unlock()
 		return nil
 	}
 	*b.val = val
+	b.lock.Unlock()
 
 	b.trigger()
 	return nil
@@ -176,12 +179,13 @@ type boundExternalBytes struct {
 
 func (b *boundExternalBytes) Set(val []byte) error {
 	b.lock.Lock()
-	defer b.lock.Unlock()
 	if bytes.Equal(b.old, val) {
+		b.lock.Unlock()
 		return nil
 	}
 	*b.val = val
 	b.old = val
+	b.lock.Unlock()
 
 	b.trigger()
 	return nil
@@ -249,11 +253,12 @@ func (b *boundFloat) Get() (float64, error) {
 
 func (b *boundFloat) Set(val float64) error {
 	b.lock.Lock()
-	defer b.lock.Unlock()
 	if *b.val == val {
+		b.lock.Unlock()
 		return nil
 	}
 	*b.val = val
+	b.lock.Unlock()
 
 	b.trigger()
 	return nil
@@ -267,12 +272,13 @@ type boundExternalFloat struct {
 
 func (b *boundExternalFloat) Set(val float64) error {
 	b.lock.Lock()
-	defer b.lock.Unlock()
 	if b.old == val {
+		b.lock.Unlock()
 		return nil
 	}
 	*b.val = val
 	b.old = val
+	b.lock.Unlock()
 
 	b.trigger()
 	return nil
@@ -340,11 +346,12 @@ func (b *boundInt) Get() (int, error) {
 
 func (b *boundInt) Set(val int) error {
 	b.lock.Lock()
-	defer b.lock.Unlock()
 	if *b.val == val {
+		b.lock.Unlock()
 		return nil
 	}
 	*b.val = val
+	b.lock.Unlock()
 
 	b.trigger()
 	return nil
@@ -358,12 +365,13 @@ type boundExternalInt struct {
 
 func (b *boundExternalInt) Set(val int) error {
 	b.lock.Lock()
-	defer b.lock.Unlock()
 	if b.old == val {
+		b.lock.Unlock()
 		return nil
 	}
 	*b.val = val
 	b.old = val
+	b.lock.Unlock()
 
 	b.trigger()
 	return nil
@@ -431,11 +439,12 @@ func (b *boundRune) Get() (rune, error) {
 
 func (b *boundRune) Set(val rune) error {
 	b.lock.Lock()
-	defer b.lock.Unlock()
 	if *b.val == val {
+		b.lock.Unlock()
 		return nil
 	}
 	*b.val = val
+	b.lock.Unlock()
 
 	b.trigger()
 	return nil
@@ -449,12 +458,13 @@ type boundExternalRune struct {
 
 func (b *boundExternalRune) Set(val rune) error {
 	b.lock.Lock()
-	defer b.lock.Unlock()
 	if b.old == val {
+		b.lock.Unlock()
 		return nil
 	}
 	*b.val = val
 	b.old = val
+	b.lock.Unlock()
 
 	b.trigger()
 	return nil
@@ -522,11 +532,12 @@ func (b *boundString) Get() (string, error) {
 
 func (b *boundString) Set(val string) error {
 	b.lock.Lock()
-	defer b.lock.Unlock()
 	if *b.val == val {
+		b.lock.Unlock()
 		return nil
 	}
 	*b.val = val
+	b.lock.Unlock()
 
 	b.trigger()
 	return nil
@@ -540,12 +551,13 @@ type boundExternalString struct {
 
 func (b *boundExternalString) Set(val string) error {
 	b.lock.Lock()
-	defer b.lock.Unlock()
 	if b.old == val {
+		b.lock.Unlock()
 		return nil
 	}
 	*b.val = val
 	b.old = val
+	b.lock.Unlock()
 
 	b.trigger()
 	return nil
@@ -613,11 +625,12 @@ func (b *boundURI) Get() (fyne.URI, error) {
 
 func (b *boundURI) Set(val fyne.URI) error {
 	b.lock.Lock()
-	defer b.lock.Unlock()
 	if compareURI(*b.val, val) {
+		b.lock.Unlock()
 		return nil
 	}
 	*b.val = val
+	b.lock.Unlock()
 
 	b.trigger()
 	return nil
@@ -631,12 +644,13 @@ type boundExternalURI struct {
 
 func (b *boundExternalURI) Set(val fyne.URI) error {
 	b.lock.Lock()
-	defer b.lock.Unlock()
 	if compareURI(b.old, val) {
+		b.lock.Unlock()
 		return nil
 	}
 	*b.val = val
 	b.old = val
+	b.lock.Unlock()
 
 	b.trigger()
 	return nil

diff --git a/data/binding/binditems_extend_test.go b/data/binding/binditems_extend_test.go
@@ -19,22 +19,19 @@ func TestBindTime(t *testing.T) {
 		called = true
 	})
 	f.AddListener(fn)
-	waitForItems()
 	assert.True(t, called)
 
 	newTime := val.Add(time.Hour)
 	called = false
 	err = f.Set(newTime)
 	assert.Nil(t, err)
-	waitForItems()
 	assert.Equal(t, newTime.Unix(), val.Unix())
 	assert.True(t, called)
 
 	newTime = newTime.Add(time.Minute)
 	called = false
 	val = newTime
 	_ = f.Reload()
-	waitForItems()
 	assert.True(t, called)
 	v, err = f.Get()
 	assert.Nil(t, err)