Possible deadlocks around BaseWidget.Theme access

[dweymouth]

Checklist

• I have searched the issue tracker for open issues that relate to the same problem, before opening a new one.
• This issue only relates to a single bug. I will open new issues for any other problems.

Describe the bug

There are at least two possible deadlocks with accessing widget themes on develop. One will (maybe?) be fixed by #4901 where buttons could deadlock when pressed, but the mobile test runner for that PR discovered another deadlock around AppTabs renderer and BaseWidget.Refresh

How to reproduce

Not sure. But here is the state of the locked goroutines from the test run:

panic: test timed out after 10m0s

goroutine 47 [running]:
testing.(*M).startAlarm.func1()
/home/runner/work/fyne/setup-go-faster/go/1.19.13/x64/src/testing/testing.go:2036 +0x8e
created by time.goFunc
/home/runner/work/fyne/setup-go-faster/go/1.19.13/x64/src/time/sleep.go:176 +0x32

goroutine 1 [chan receive, 9 minutes]:
testing.(*T).Run(0xc0001411e0, {0x95fb4b?, 0x4e94c5?}, 0x994be8)
/home/runner/work/fyne/setup-go-faster/go/1.19.13/x64/src/testing/testing.go:1494 +0x37a
testing.runTests.func1(0xc0001411e0?)
/home/runner/work/fyne/setup-go-faster/go/1.19.13/x64/src/testing/testing.go:1846 +0x6e
testing.tRunner(0xc0001411e0, 0xc00029fcd8)
/home/runner/work/fyne/setup-go-faster/go/1.19.13/x64/src/testing/testing.go:1446 +0x10b
testing.runTests(0xc0002e40a0?, {0xcd72e0, 0x46, 0x46}, {0x7f96027c0108?, 0x40?, 0x1420f20?})
/home/runner/work/fyne/setup-go-faster/go/1.19.13/x64/src/testing/testing.go:1844 +0x456
testing.(*M).Run(0xc0002e40a0)
/home/runner/work/fyne/setup-go-faster/go/1.19.13/x64/src/testing/testing.go:1726 +0x5d9
main.main()
_testmain.go:187 +0x1aa

goroutine 6 [chan receive, 10 minutes]:
fyne.io/fyne/v2/test.NewApp.func1()
/home/runner/work/fyne/fyne/test/testapp.go:166 +0x45
created by fyne.io/fyne/v2/test.NewApp
/home/runner/work/fyne/fyne/test/testapp.go:164 +0x2ed

goroutine 23 [semacquire, 9 minutes]:
sync.runtime_SemacquireMutex(0x0?, 0xa0?, 0xc001cec780?)
/home/runner/work/fyne/setup-go-faster/go/1.19.13/x64/src/runtime/sema.go:77 +0x25
sync.(*RWMutex).RLock(...)
/home/runner/work/fyne/setup-go-faster/go/1.19.13/x64/src/sync/rwmutex.go:71
fyne.io/fyne/v2/widget.(*BaseWidget).Visible(0xc001cec780?)
/home/runner/work/fyne/fyne/widget/widget.go:85 +0x53
fyne.io/fyne/v2/widget.(*buttonRenderer).updateIconAndText(0xc000130dc0)
/home/runner/work/fyne/fyne/widget/button.go:396 +0x34
fyne.io/fyne/v2/widget.(*buttonRenderer).Refresh(0xc000130dc0)
/home/runner/work/fyne/fyne/widget/button.go:308 +0xd6
fyne.io/fyne/v2/widget.(*BaseWidget).Refresh(0xc001cec780)
/home/runner/work/fyne/fyne/widget/widget.go:137 +0xb3
fyne.io/fyne/v2.(*Container).Refresh(0xc001c46550)
/home/runner/work/fyne/fyne/container.go:117 +0xe2
fyne.io/fyne/v2/container.(*baseTabsRenderer).refresh(0xc001c19480, {0xa59500?, 0xc001cec6e0?})
/home/runner/work/fyne/fyne/container/tabs.go:480 +0x36
fyne.io/fyne/v2/container.(*appTabsRenderer).Refresh(0xc001c19480)
/home/runner/work/fyne/fyne/container/apptabs.go:318 +0x4e
fyne.io/fyne/v2/widget.(*BaseWidget).Refresh(0xc001cec6e0)
/home/runner/work/fyne/fyne/widget/widget.go:137 +0xb3
fyne.io/fyne/v2/container.(*AppTabs).SelectIndex(0xc001cec6e0, 0xa57288?)
/home/runner/work/fyne/fyne/container/apptabs.go:167 +0x37
fyne.io/fyne/v2/container.TestTab_ThemeChange(0xc001c44d00?)
/home/runner/work/fyne/fyne/container/tabs_test.go:42 +0x34f
testing.tRunner(0xc001bd2d00, 0x994be8)
/home/runner/work/fyne/setup-go-faster/go/1.19.13/x64/src/testing/testing.go:1446 +0x10b
created by testing.(*T).Run
/home/runner/work/fyne/setup-go-faster/go/1.19.13/x64/src/testing/testing.go:1493 +0x35f

goroutine 24 [semacquire, 9 minutes]:
sync.runtime_SemacquireMutex(0xc0020a6b00?, 0x70?, 0x0?)
/home/runner/work/fyne/setup-go-faster/go/1.19.13/x64/src/runtime/sema.go:77 +0x25
sync.(*RWMutex).Lock(0x1?)
/home/runner/work/fyne/setup-go-faster/go/1.19.13/x64/src/sync/rwmutex.go:152 +0x71
fyne.io/fyne/v2/widget.(*BaseWidget).Refresh(0xc001cec780)
/home/runner/work/fyne/fyne/widget/widget.go:132 +0x66
fyne.io/fyne/v2/internal/app.ApplyThemeTo({0xa59b60?, 0xc001cec780?}, {0xa5bbf8?, 0xc0020a6b00?})
/home/runner/work/fyne/fyne/internal/app/theme.go:29 +0x123
fyne.io/fyne/v2/internal/app.ApplyThemeTo({0xa59140, 0xc001c46550}, {0xa5bbf8, 0xc0020a6b00})
/home/runner/work/fyne/fyne/internal/app/theme.go:23 +0x245
fyne.io/fyne/v2/internal/app.ApplyThemeTo({0xa594a0, 0xc001cec6e0}, {0xa5bbf8, 0xc0020a6b00})
/home/runner/work/fyne/fyne/internal/app/theme.go:18 +0x1b3
fyne.io/fyne/v2/internal/app.ApplySettingsWithCallback({0xc001cec660?, 0xc001bd2d00?}, {0xa5b938?, 0xc001cec640?}, 0x0)
/home/runner/work/fyne/fyne/internal/app/theme.go:43 +0xca
fyne.io/fyne/v2/internal/app.ApplySettings(...)
/home/runner/work/fyne/fyne/internal/app/theme.go:35
fyne.io/fyne/v2/test.NewApp.func1()
/home/runner/work/fyne/fyne/test/testapp.go:170 +0xad
created by fyne.io/fyne/v2/test.NewApp
/home/runner/work/fyne/fyne/test/testapp.go:164 +0x2ed
FAIL fyne.io/fyne/v2/container 600.112s

Screenshots

No response

Example code

mobile tests

Fyne version

develop

Go compiler version

n/a

Operating system and version

mobile test runner GH actions

Additional Information

No response

[andydotxyz]

Looks like The Button used in the tabs updateIconAndText has a lock but it calls Visible which is supposed to be called unlocked?

[dweymouth]

That is what was fixed in the merged PR - Visible can be called unlocked but also can be called with RLock, since it acquires RLock itself to check the widget theme (the fix was that Visible used to acquire Lock, not RLock). And the button calls updateIconAndText with the RLock (or unlocked in the constroctor), never with Lock.

I think the above stacktrace relates to a race condition with ApplyThemeTo. Though I have not seen a deadlock with that stack trace in real use on desktop - the PR that was merged seems to have fixed all the deadlocking around buttons I'd seen in practice.

[andydotxyz]

Great to hear. Should this be closed then?

[dweymouth]

Well I don't think so, since the above locked stacktrace happened with the PR applied... unless we can prove it only comes up in test code

[dweymouth]

We probably can remove the blocker from this issue however and treat it like the other races tickets in the backlog

[andydotxyz]

Let's review with latest develop - I have not seen any deadlocks since two that were found have been fixed on the branch.

[andydotxyz]

I'm glad the deadlock has gone seemingly - what is the remaining race needing to keep this open?

[dweymouth]

The stack trace in this ticket was from a test runner on the PR that had the button deadlock fix. So it shows there's another potential deadlock that can be triggered, though perhaps only in test code. I'd say we should keep this open until we can track it down, or prove it only comes up in tests, or we have provably eliminated all races with whatever the 2.6 threading model will be