Cogent Core demo crash - Githubissues

arturbasinki commented 2 months ago

Describe the bug

demo app crashes on linux

How to reproduce

App crashes during testing Collections

Example code

No response

Relevant output

Crash log saved in /home/airbass/.config/Cogent\ Core\ Demo/crash-logs

Platform: Linux
System platform: Linux
App version: dev
Core version: dev
Time: 2024-07-31 11:56:53

panic: reflect: NumField of non-struct type string

goroutine 74 [running]:
runtime/debug.Stack()
    /usr/local/go/src/runtime/debug/stack.go:24 +0x5e
cogentcore.org/core/core.handleRecover({0xe0e760, 0xc007f1be20})
    /home/airbass/go/pkg/mod/cogentcore.org/core@v0.3.1-0.20240731002359-25447b5a3c02/core/recover.go:38 +0x6c
cogentcore.org/core/core.(*renderWindow).eventLoop.func1()
    /home/airbass/go/pkg/mod/cogentcore.org/core@v0.3.1-0.20240731002359-25447b5a3c02/core/renderwindow.go:393 +0x24
panic({0xe0e760?, 0xc007f1be20?})
    /usr/local/go/src/runtime/panic.go:770 +0x132
reflect.(*rtype).NumField(0xdf2040?)
    /usr/local/go/src/reflect/type.go:769 +0x59
cogentcore.org/core/base/reflectx.walkFields({0xe0e760?, 0xc001fa3780?, 0x711947?}, 0x0, 0xc00004f2e8, 0xc00004f2d0)
    /home/airbass/go/pkg/mod/cogentcore.org/core@v0.3.1-0.20240731002359-25447b5a3c02/base/reflectx/structs.go:36 +0xf3
cogentcore.org/core/base/reflectx.WalkFields(...)
    /home/airbass/go/pkg/mod/cogentcore.org/core@v0.3.1-0.20240731002359-25447b5a3c02/base/reflectx/structs.go:31
cogentcore.org/core/core.(*Form).getStructFields(0xc0029d0008)
    /home/airbass/go/pkg/mod/cogentcore.org/core@v0.3.1-0.20240731002359-25447b5a3c02/core/form.go:92 +0x115
cogentcore.org/core/core.(*Form).Init.func2(0xc002b46ba0)
    /home/airbass/go/pkg/mod/cogentcore.org/core@v0.3.1-0.20240731002359-25447b5a3c02/core/form.go:136 +0x5c
cogentcore.org/core/tree.(*NodeBase).Make.func1({0xc00050e740, 0x1, 0x0?})
    /home/airbass/go/pkg/mod/cogentcore.org/core@v0.3.1-0.20240731002359-25447b5a3c02/tree/plan.go:103 +0x39
cogentcore.org/core/base/tiered.(*Tiered[...]).Do(0x416f05, 0xc00004f508?)
    /home/airbass/go/pkg/mod/cogentcore.org/core@v0.3.1-0.20240731002359-25447b5a3c02/base/tiered/tiered.go:30 +0x4c
cogentcore.org/core/tree.(*NodeBase).Make(0x3?, 0x2?)
    /home/airbass/go/pkg/mod/cogentcore.org/core@v0.3.1-0.20240731002359-25447b5a3c02/tree/plan.go:101 +0x5e
cogentcore.org/core/tree.(*NodeBase).UpdateFromMake(0xc0029d0008)
    /home/airbass/go/pkg/mod/cogentcore.org/core@v0.3.1-0.20240731002359-25447b5a3c02/tree/plan.go:111 +0x71
cogentcore.org/core/core.(*WidgetBase).UpdateWidget.(*NodeBase).RunUpdaters.func1({0xc0029cbbf0, 0xf5e4e0?, 0x0?})
    /home/airbass/go/pkg/mod/cogentcore.org/core@v0.3.1-0.20240731002359-25447b5a3c02/tree/plan.go:121 +0x26
cogentcore.org/core/base/tiered.(*Tiered[...]).Do(0xf5e4e0, 0x1396928?)
    /home/airbass/go/pkg/mod/cogentcore.org/core@v0.3.1-0.20240731002359-25447b5a3c02/base/tiered/tiered.go:30 +0x4c
cogentcore.org/core/tree.(*NodeBase).RunUpdaters(...)
    /home/airbass/go/pkg/mod/cogentcore.org/core@v0.3.1-0.20240731002359-25447b5a3c02/tree/plan.go:119
cogentcore.org/core/core.(*WidgetBase).UpdateWidget(0xc0029d0008)
    /home/airbass/go/pkg/mod/cogentcore.org/core@v0.3.1-0.20240731002359-25447b5a3c02/core/update.go:25 +0x45
cogentcore.org/core/core.(*WidgetBase).Update.func1({0x724c1d687d68, 0xc0029d0008}, 0xc00004f6a8?)
    /home/airbass/go/pkg/mod/cogentcore.org/core@v0.3.1-0.20240731002359-25447b5a3c02/core/update.go:61 +0x25
cogentcore.org/core/core.(*WidgetBase).Update.(*WidgetBase).WidgetWalkDown.func2({0x724c1d687d18, 0xc0029d0008})
    /home/airbass/go/pkg/mod/cogentcore.org/core@v0.3.1-0.20240731002359-25447b5a3c02/core/widget.go:449 +0x9c
cogentcore.org/core/tree.(*NodeBase).WalkDown(0xc0029c3508, 0xc007b007a0)
    /home/airbass/go/pkg/mod/cogentcore.org/core@v0.3.1-0.20240731002359-25447b5a3c02/tree/nodebase.go:500 +0x12b
cogentcore.org/core/core.(*WidgetBase).WidgetWalkDown(...)
    /home/airbass/go/pkg/mod/cogentcore.org/core@v0.3.1-0.20240731002359-25447b5a3c02/core/widget.go:447
cogentcore.org/core/core.(*WidgetBase).Update(0xc0029c3508)
    /home/airbass/go/pkg/mod/cogentcore.org/core@v0.3.1-0.20240731002359-25447b5a3c02/core/update.go:60 +0xad
cogentcore.org/core/core.(*Form).Init.func2.3.2({0x14de1d0?, 0xc002b40c40?})
    /home/airbass/go/pkg/mod/cogentcore.org/core@v0.3.1-0.20240731002359-25447b5a3c02/core/form.go:240 +0x90
cogentcore.org/core/events.(*Listeners).Call(0xc00004f910, {0x14de1d0, 0xc002b40c40}, {0xc00004f8f8, 0x1, 0x66aa0a5b?})
    /home/airbass/go/pkg/mod/cogentcore.org/core@v0.3.1-0.20240731002359-25447b5a3c02/events/listeners.go:61 +0xfd
cogentcore.org/core/core.(*WidgetBase).HandleEvent.func1(0xc00193d140)
    /home/airbass/go/pkg/mod/cogentcore.org/core@v0.3.1-0.20240731002359-25447b5a3c02/core/widgetevents.go:244 +0x65
cogentcore.org/core/base/tiered.(*Tiered[...]).Do(0x214cccd, 0xc00004f988?)
    /home/airbass/go/pkg/mod/cogentcore.org/core@v0.3.1-0.20240731002359-25447b5a3c02/base/tiered/tiered.go:30 +0x39
cogentcore.org/core/core.(*WidgetBase).HandleEvent(0xc0029cca88, {0x14de1d0?, 0xc002b40c40?})
    /home/airbass/go/pkg/mod/cogentcore.org/core@v0.3.1-0.20240731002359-25447b5a3c02/core/widgetevents.go:243 +0x168
cogentcore.org/core/core.(*WidgetBase).Send(0xc0029cca88, 0x24, {0xc00004fa18?, 0xea?, 0x1?})
    /home/airbass/go/pkg/mod/cogentcore.org/core@v0.3.1-0.20240731002359-25447b5a3c02/core/widgetevents.go:180 +0x77
cogentcore.org/core/core.(*WidgetBase).SendChange(...)
    /home/airbass/go/pkg/mod/cogentcore.org/core@v0.3.1-0.20240731002359-25447b5a3c02/core/widgetevents.go:187
cogentcore.org/core/core.(*Switch).Init.func2({0x14de1d0, 0xc002b40a80})
    /home/airbass/go/pkg/mod/cogentcore.org/core@v0.3.1-0.20240731002359-25447b5a3c02/core/switch.go:133 +0xa6
cogentcore.org/core/events.(*Listeners).Call(0xc00004fac8, {0x14de1d0, 0xc002b40a80}, {0xc00004fab0, 0x1, 0x66aa0a5b?})
    /home/airbass/go/pkg/mod/cogentcore.org/core@v0.3.1-0.20240731002359-25447b5a3c02/events/listeners.go:61 +0xfd
cogentcore.org/core/core.(*WidgetBase).HandleEvent.func1(0xc00193d1a0)
    /home/airbass/go/pkg/mod/cogentcore.org/core@v0.3.1-0.20240731002359-25447b5a3c02/core/widgetevents.go:244 +0x65
cogentcore.org/core/base/tiered.(*Tiered[...]).Do(0x214cccd, 0xc00004fb40?)
    /home/airbass/go/pkg/mod/cogentcore.org/core@v0.3.1-0.20240731002359-25447b5a3c02/base/tiered/tiered.go:31 +0x4c
cogentcore.org/core/core.(*WidgetBase).HandleEvent(0xc0029cca88, {0x14de1d0?, 0xc002b40a80?})
    /home/airbass/go/pkg/mod/cogentcore.org/core@v0.3.1-0.20240731002359-25447b5a3c02/core/widgetevents.go:243 +0x168
cogentcore.org/core/core.(*WidgetBase).Send(0xc0029cca88, 0x5, {0xc00004fdf0?, 0x10002b3f8d0?, 0x724c140a5b58?})
    /home/airbass/go/pkg/mod/cogentcore.org/core@v0.3.1-0.20240731002359-25447b5a3c02/core/widgetevents.go:180 +0x77
cogentcore.org/core/core.(*Events).handlePosEvent(0xc0001b6b00, {0x14de450, 0xc003921c00})
    /home/airbass/go/pkg/mod/cogentcore.org/core@v0.3.1-0.20240731002359-25447b5a3c02/core/events.go:471 +0x209e
cogentcore.org/core/core.(*Events).handleEvent(0xc0001b6b00, {0x14de450, 0xc003921c00})
    /home/airbass/go/pkg/mod/cogentcore.org/core@v0.3.1-0.20240731002359-25447b5a3c02/core/events.go:169 +0x58
cogentcore.org/core/core.(*Stage).mainHandleEvent(0xc0031314a0, {0x14de450, 0xc003921c00})
    /home/airbass/go/pkg/mod/cogentcore.org/core@v0.3.1-0.20240731002359-25447b5a3c02/core/mainstage.go:372 +0x170
cogentcore.org/core/core.(*stages).mainHandleEvent(0xc003f92100, {0x14de450, 0xc003921c00})
    /home/airbass/go/pkg/mod/cogentcore.org/core@v0.3.1-0.20240731002359-25447b5a3c02/core/mainstage.go:380 +0x6e
cogentcore.org/core/core.(*renderWindow).handleEvent(0xc003f920d0, {0x14de450, 0xc003921c00})
    /home/airbass/go/pkg/mod/cogentcore.org/core@v0.3.1-0.20240731002359-25447b5a3c02/core/renderwindow.go:444 +0x15e
cogentcore.org/core/core.(*renderWindow).eventLoop(0xc003f920d0)
    /home/airbass/go/pkg/mod/cogentcore.org/core@v0.3.1-0.20240731002359-25447b5a3c02/core/renderwindow.go:407 +0x87
created by cogentcore.org/core/core.(*renderWindow).goStartEventLoop in goroutine 1
    /home/airbass/go/pkg/mod/cogentcore.org/core@v0.3.1-0.20240731002359-25447b5a3c02/core/renderwindow.go:378 +0x65

Platform

Linux

kkoreilly commented 2 months ago

Thank you for reporting this! I can reproduce this and will work on fixing it.

kkoreilly commented 2 months ago

A summary of the situation here:

This issue occurs when you trigger a ShouldDisplayer condition that was not previously satisfied in a Form
When we make the children of a form, we loop through the fields, getting i, f from the range loop
We use i to re-capture f in inner closures to avoid f becoming stale
However, i is actually the one that becomes stale, since the field index is different once a conditional changes
And f should never become stale since the name is based on both the field path and the field type name, which should only both be the same if it is actually the same field
(Although if someone sets the struct of a Form to a different struct of the same type, the f will indeed be stale, since it has the same name and type, but it is a different actual field, and thus the value is never updated)
~This is actually potentially the cause of #1027~ (This is actually not the cause of #1027, but if we did use f instead of i in the value widget Updater, we would have a bunch of issues like that, so that is why we are using i there)
In summary, our Form plan updating logic relies on both i and f at different points, but neither is actually robust with our current structure.
In fact, we use f extensively in the label closures, which is predicated on false assumptions and means that there are ways to break that logic (for example, if you set the struct of a Form to a struct that is otherwise the same but has different default struct tags or different documentation, it will not update accordingly; that is an exceedingly rare possibility and not worth fixing now; see #1098)
One option for resolving all of these issues is to add i to the name in the plan, ensuring that it is never stale, and then always use i as the ground truth and never f. That approach is somewhat similar to what we do elsewhere and probably the best way forward at this point.

@rcoreilly any thoughts? Good for me to move forward with the plan stated in the last bullet point?

rcoreilly commented 2 months ago

last option sounds fine to me..

cogentcore / core

Cogent Core demo crash #1096

Describe the bug

How to reproduce

Example code

Relevant output

Platform