fyne icon indicating copy to clipboard operation
fyne copied to clipboard

Clean up locking in canvas.EnsureMinSize()

Open Jacalz opened this issue 2 years ago • 5 comments

Checklist

  • [X] I have searched the issue tracker for open issues that relate to the same feature, before opening a new one.
  • [X] This issue only relates to a single feature. I will open new issues for any other features.

Is your feature request related to a problem?

The canvas.EnsureMinSize() method seems to have caused the deadlock in https://github.com/fyne-io/fyne/issues/4086 and is generally very complex. It holds a read lock when traversing down the object tree and then has to unlock and then lock again after calling something that might invoke the lock.

This kind of reverse locking seems very complicated and like a possible source of bugs. It would be good if we could clean up the locking and the method to avoid these kind of issues.

Is it possible to construct a solution with the existing API?

I think so

Describe the solution you'd like to see.

If we could reduce the amount of locking, or potentially reverse the locking to lock and then unlock (instead of unlock and then lock) we can hopefully reduce the risk of deadlocks and gain better performance by avoiding some lock contention.

Jacalz avatar Aug 24 '23 10:08 Jacalz

This keeps happening occasionnally on 2.4.4

"sync: RUnlock of unlocked RWMutex"
Stack:
	 1  0x00007ff668e8b25e in sync.fatal
	     at C:/Users/Matwachich/sdk/go1.20.7/src/runtime/panic.go:1031
	 2  0x00007ff668ea53bd in sync.(*RWMutex).rUnlockSlow
	     at C:/Users/Matwachich/sdk/go1.20.7/src/sync/rwmutex.go:129
	 3  0x00007ff668ea5327 in sync.(*RWMutex).RUnlock
	     at C:/Users/Matwachich/sdk/go1.20.7/src/sync/rwmutex.go:119
	 4  0x00007ff669c0d76b in fyne.io/fyne/v2/internal/driver/common.(*Canvas).EnsureMinSize.func2
	     at C:/Users/Matwachich/go/pkg/mod/fyne.io/fyne/[email protected]/internal/driver/common/canvas.go:95
	 7  0x00007ff669b63f65 in github.com/matwachich/emr-made-simple/libs/commonwidgets.(*mediaListItem).update
	     at c:/Users/Matwachich/Documents/Programmation/EMR Made Simple/emr-made-simple/libs/commonwidgets/media_list.go:800
	 8  0x00007ff669b90a0a in github.com/matwachich/emr-made-simple/libs/commonwidgets.NewMediaList.func11
	     at c:/Users/Matwachich/Documents/Programmation/EMR Made Simple/emr-made-simple/libs/commonwidgets/media_list.go:156
	 9  0x00007ff6698b7499 in fyne.io/fyne/v2/widget.(*listLayout).setupListItem
	     at C:/Users/Matwachich/go/pkg/mod/fyne.io/fyne/[email protected]/widget/list.go:625
	10  0x00007ff6698b8865 in fyne.io/fyne/v2/widget.(*listLayout).updateList
	     at C:/Users/Matwachich/go/pkg/mod/fyne.io/fyne/[email protected]/widget/list.go:726
	11  0x00007ff6698b4926 in fyne.io/fyne/v2/widget.(*List).Resize
	     at C:/Users/Matwachich/go/pkg/mod/fyne.io/fyne/[email protected]/widget/list.go:193
	12  0x00007ff66953ca02 in fyne.io/fyne/v2/layout.(*borderLayout).Layout
	     at C:/Users/Matwachich/go/pkg/mod/fyne.io/fyne/[email protected]/layout/borderlayout.go:61
	13  0x00007ff669c10c8e in fyne.io/fyne/v2/internal/driver/common.(*Canvas).updateLayout
	     at C:/Users/Matwachich/go/pkg/mod/fyne.io/fyne/[email protected]/internal/driver/common/canvas.go:601
	14  0x00007ff669c0d306 in fyne.io/fyne/v2/internal/driver/common.(*Canvas).EnsureMinSize.func1
	     at C:/Users/Matwachich/go/pkg/mod/fyne.io/fyne/[email protected]/internal/driver/common/canvas.go:110
	15  0x00007ff669c0ff37 in fyne.io/fyne/v2/internal/driver/common.(*Canvas).walkTree.func2
	     at C:/Users/Matwachich/go/pkg/mod/fyne.io/fyne/[email protected]/internal/driver/common/canvas.go:518
	16  0x00007ff66990e74d in fyne.io/fyne/v2/internal/driver.walkObjectTree
	     at C:/Users/Matwachich/go/pkg/mod/fyne.io/fyne/[email protected]/internal/driver/util.go:199
	17  0x00007ff66990e9cb in fyne.io/fyne/v2/internal/driver.walkObjectTree.func1
	     at C:/Users/Matwachich/go/pkg/mod/fyne.io/fyne/[email protected]/internal/driver/util.go:178
	18  0x00007ff66990e6dd in fyne.io/fyne/v2/internal/driver.walkObjectTree
	     at C:/Users/Matwachich/go/pkg/mod/fyne.io/fyne/[email protected]/internal/driver/util.go:192
	19  0x00007ff66990e9cb in fyne.io/fyne/v2/internal/driver.walkObjectTree.func1
	     at C:/Users/Matwachich/go/pkg/mod/fyne.io/fyne/[email protected]/internal/driver/util.go:178
	20  0x00007ff66990e6dd in fyne.io/fyne/v2/internal/driver.walkObjectTree
	     at C:/Users/Matwachich/go/pkg/mod/fyne.io/fyne/[email protected]/internal/driver/util.go:192
	21  0x00007ff66990e9cb in fyne.io/fyne/v2/internal/driver.walkObjectTree.func1
	     at C:/Users/Matwachich/go/pkg/mod/fyne.io/fyne/[email protected]/internal/driver/util.go:178
	22  0x00007ff66990e6dd in fyne.io/fyne/v2/internal/driver.walkObjectTree
	     at C:/Users/Matwachich/go/pkg/mod/fyne.io/fyne/[email protected]/internal/driver/util.go:192
	23  0x00007ff66990e9cb in fyne.io/fyne/v2/internal/driver.walkObjectTree.func1
	     at C:/Users/Matwachich/go/pkg/mod/fyne.io/fyne/[email protected]/internal/driver/util.go:178
	24  0x00007ff66990e6dd in fyne.io/fyne/v2/internal/driver.walkObjectTree
	     at C:/Users/Matwachich/go/pkg/mod/fyne.io/fyne/[email protected]/internal/driver/util.go:192
	25  0x00007ff66990e9cb in fyne.io/fyne/v2/internal/driver.walkObjectTree.func1
	     at C:/Users/Matwachich/go/pkg/mod/fyne.io/fyne/[email protected]/internal/driver/util.go:178
	26  0x00007ff66990e6dd in fyne.io/fyne/v2/internal/driver.walkObjectTree
	     at C:/Users/Matwachich/go/pkg/mod/fyne.io/fyne/[email protected]/internal/driver/util.go:192
	27  0x00007ff66990e9cb in fyne.io/fyne/v2/internal/driver.walkObjectTree.func1
	     at C:/Users/Matwachich/go/pkg/mod/fyne.io/fyne/[email protected]/internal/driver/util.go:178
	28  0x00007ff66990e6dd in fyne.io/fyne/v2/internal/driver.walkObjectTree
	     at C:/Users/Matwachich/go/pkg/mod/fyne.io/fyne/[email protected]/internal/driver/util.go:192
	29  0x00007ff66990e9cb in fyne.io/fyne/v2/internal/driver.walkObjectTree.func1
	     at C:/Users/Matwachich/go/pkg/mod/fyne.io/fyne/[email protected]/internal/driver/util.go:178
	30  0x00007ff66990e6dd in fyne.io/fyne/v2/internal/driver.walkObjectTree
	     at C:/Users/Matwachich/go/pkg/mod/fyne.io/fyne/[email protected]/internal/driver/util.go:192
	31  0x00007ff66990e9cb in fyne.io/fyne/v2/internal/driver.walkObjectTree.func1
	     at C:/Users/Matwachich/go/pkg/mod/fyne.io/fyne/[email protected]/internal/driver/util.go:178
	32  0x00007ff66990e6dd in fyne.io/fyne/v2/internal/driver.walkObjectTree
	     at C:/Users/Matwachich/go/pkg/mod/fyne.io/fyne/[email protected]/internal/driver/util.go:192
	33  0x00007ff66990df85 in fyne.io/fyne/v2/internal/driver.WalkVisibleObjectTree
	     at C:/Users/Matwachich/go/pkg/mod/fyne.io/fyne/[email protected]/internal/driver/util.go:134
	34  0x00007ff669c0fd8b in fyne.io/fyne/v2/internal/driver/common.(*Canvas).walkTree
	     at C:/Users/Matwachich/go/pkg/mod/fyne.io/fyne/[email protected]/internal/driver/common/canvas.go:524
	35  0x00007ff669c0f57b in fyne.io/fyne/v2/internal/driver/common.(*Canvas).WalkTrees
	     at C:/Users/Matwachich/go/pkg/mod/fyne.io/fyne/[email protected]/internal/driver/common/canvas.go:436
	36  0x00007ff669c0cff6 in fyne.io/fyne/v2/internal/driver/common.(*Canvas).EnsureMinSize
	     at C:/Users/Matwachich/go/pkg/mod/fyne.io/fyne/[email protected]/internal/driver/common/canvas.go:143
	37  0x00007ff669c4087b in fyne.io/fyne/v2/internal/driver/glfw.(*gLDriver).repaintWindow.func1
	     at C:/Users/Matwachich/go/pkg/mod/fyne.io/fyne/[email protected]/internal/driver/glfw/loop.go:215
	38  0x00007ff669c4d939 in fyne.io/fyne/v2/internal/driver/glfw.(*window).RunWithContext
	     at C:/Users/Matwachich/go/pkg/mod/fyne.io/fyne/[email protected]/internal/driver/glfw/window.go:923
	39  0x00007ff669c40812 in fyne.io/fyne/v2/internal/driver/glfw.(*gLDriver).repaintWindow
	     at C:/Users/Matwachich/go/pkg/mod/fyne.io/fyne/[email protected]/internal/driver/glfw/loop.go:214
	40  0x00007ff669c3fc85 in fyne.io/fyne/v2/internal/driver/glfw.(*gLDriver).drawSingleFrame
	     at C:/Users/Matwachich/go/pkg/mod/fyne.io/fyne/[email protected]/internal/driver/glfw/loop.go:102
	41  0x00007ff669c40c6f in fyne.io/fyne/v2/internal/driver/glfw.(*gLDriver).startDrawThread.func1
	     at C:/Users/Matwachich/go/pkg/mod/fyne.io/fyne/[email protected]/internal/driver/glfw/loop.go:270

matwachich avatar Mar 18 '24 09:03 matwachich

From what I read above it sounds like this will be solved by the new threading model, but may be too hard to solve earlier than that.

andydotxyz avatar Apr 01 '24 20:04 andydotxyz

I have code that triggers this bug every time (5/5 so far). Any workaround?

pierrec avatar Sep 16 '24 08:09 pierrec

I have code that triggers this bug every time (5/5 so far). Any workaround?

If you could share the reproduction steps / code so we can replicate it locally that would be very helpful.

andydotxyz avatar Sep 16 '24 09:09 andydotxyz

I have code that triggers this bug every time (5/5 so far). Any workaround?

If you could share the reproduction steps / code so we can replicate it locally that would be very helpful.

Yes I will. Having issues with codeberg right now, will try later on or tomorrow.

pierrec avatar Sep 16 '24 13:09 pierrec