Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

stack overflow when calling SetRowHeight in table UpdateCell callback #5007

Closed
2 tasks done
MidnightSong opened this issue Jul 18, 2024 · 8 comments · May be fixed by JanVaya/fyne#1
Closed
2 tasks done

stack overflow when calling SetRowHeight in table UpdateCell callback #5007

MidnightSong opened this issue Jul 18, 2024 · 8 comments · May be fixed by JanVaya/fyne#1
Labels
bug Something isn't working

Comments

@MidnightSong
Copy link

MidnightSong commented Jul 18, 2024

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

I create a table widget . But the row height is uncertain when it created, only until use other variable return fields.

runtime: goroutine stack exceeds 1000000000-byte limit
runtime: sp=0x1407a780420 stack=[0x1407a780000, 0x1409a780000]
fatal error: stack overflow

runtime stack:
runtime.throw({0x104b739ce?, 0x16ee4adb8?})
/usr/local/go/src/runtime/panic.go:1023 +0x40 fp=0x16ee4ad50 sp=0x16ee4ad20 pc=0x103016420
runtime.newstack()
/usr/local/go/src/runtime/stack.go:1103 +0x478 fp=0x16ee4aef0 sp=0x16ee4ad50 pc=0x103030fa8
runtime.morestack()
/usr/local/go/src/runtime/asm_arm64.s:341 +0x70 fp=0x16ee4aef0 sp=0x16ee4aef0 pc=0x103049010

goroutine 115 gp=0x1400f26ce00 m=5 mp=0x14000100008 [running]:
fyne.io/fyne/v2/widget.splitLines(0x140002491f0)
/Users/xjy/go/pkg/mod/fyne.io/fyne/v2@v2.5.0/widget/richtext.go:1097 +0x3e8 fp=0x1407a780420 sp=0x1407a780420 pc=0x104999418
fyne.io/fyne/v2/widget.lineBounds(0x140002491f0, 0x0, 0x0, 0x436a0000, {0x436a0000, 0x4193e000}, 0x1407a780be8)
/Users/xjy/go/pkg/mod/fyne.io/fyne/v2@v2.5.0/widget/richtext.go:943 +0x54 fp=0x1407a780a20 sp=0x1407a780420 pc=0x104997714
fyne.io/fyne/v2/widget.(*RichText).updateRowBounds.func1({0x140104a03f0, 0x1, 0x1})

when i remove this code :

table.SetRowHeight(id.Row, 50).

Everything has become normal

How to reproduce

just use i provide code

Screenshots

Longshot-20240718234817

Example code

when i use this code:

package main

import (
	"fyne.io/fyne/v2"
	"fyne.io/fyne/v2/app"
	"fyne.io/fyne/v2/container"
	"fyne.io/fyne/v2/widget"
)

var data = [][]string{
	{"this is a long string", "Row 1, Col 2"},
	{"Row 2, Col 1", "Row 2, Col 2"},
	{"Row 3, Col 1", "Row 3, Col 2"},
	{"Row 4, Col 1", "Row 4, Col 2"},
	{"Row 5, Col 1", "Row 5, Col 2"},
	{"Row 6, Col 1", "Row 6, Col 2"},
}

func main() {
	mainApp := app.NewWithID("a.b.c.d")
	mainWin := mainApp.NewWindow("test table")
	table := getTable()
	mainWin.SetContent(container.NewStack(table))
	mainWin.CenterOnScreen()
	mainWin.Resize(fyne.NewSize(600, 400))
	mainWin.ShowAndRun()
}

func getTable() *widget.Table {
	var table *widget.Table
	table = widget.NewTableWithHeaders(func() (int, int) { return len(data), 2 },
		func() fyne.CanvasObject {
			label := widget.NewLabel("unload")
			return label
		},
		func(id widget.TableCellID, cell fyne.CanvasObject) {
			c := cell.(*widget.Label)
			if len(data[id.Row][id.Col]) > 10 {
				table.SetRowHeight(id.Row, 50)
			}
			c.SetText(data[id.Row][id.Col])
		},
	)
	table.SetColumnWidth(0, 100)
	table.SetColumnWidth(1, 100)
	return table
}

Fyne version

v2.5.0

Go compiler version

1.21

Operating system and version

macOS Sonoma 14.5 Apple M2

Additional Information

No response

@MidnightSong MidnightSong added the unverified A bug that has been reported but not verified label Jul 18, 2024
@andydotxyz
Copy link
Member

Is it possible to please provide a code sample that actually replicates this issue? The snippet included does not compile.

@hfmrow
Copy link

hfmrow commented Jul 18, 2024

Hi,

Personally, I don't use 'table.SetRowHeight' inside the 'update' function because it seems logical that an issue will occur, given that the row size increases in height, it will trigger a table update again and this will create an infinite loop that will ultimately result in a stack overflow (This is what I believe, but I might be mistaken).

So, to keep it simple, you put 'table.SetRowHeight' right after creating your table, something like this:

        var table *widget.Table
       
	// ...

	// ...

	table = widget.NewTable(length, create, update)

	for i := range items.EnvItems {
		table.SetRowHeight(i, rowHeight)
	}

	return table

And it works perfectly.

Here's a complete example:

package main

import (
	"fyne.io/fyne/v2"
	"fyne.io/fyne/v2/app"
	"fyne.io/fyne/v2/container"
	"fyne.io/fyne/v2/theme"
	"fyne.io/fyne/v2/widget"
)

var (
	// Dummy data
	chats = [][]string{
		{"title0", "content0", "author0"},
		{"title1", "content1", "author1"},
		{"title2", "content2", "author2"},
		{"title3verylongwithlotofwords", "content3", "author3"},
		{"title4", "content4", "author4"},
		{"title5", "content5", "author5"},
		{"title6verylongwithlotofwords", "content6", "author6"},
		{"title7verylongwithlotofwords", "content7", "author7"},
		{"title8", "content8", "author8"},
		{"title9verylongwithlotofwords", "content9", "author9"},
	}

	// Set to false to disable text truncation.
	// It will use manual width sizing instead.
	withTruncation bool = true
)

func main() {
	mainApp := app.NewWithID("a.b.c.d")
	mainWin := mainApp.NewWindow("test table")

	table := openedDialogs()

	mainWin.SetContent(container.NewStack(table))
	mainWin.CenterOnScreen()
	mainWin.Resize(fyne.NewSize(800, 400))

	mainWin.ShowAndRun()
}

func openedDialogs() *widget.Table {
	// Get length
	length := func() (int, int) { return len(chats), 3 }

	// Create label
	create := func() fyne.CanvasObject {
		return widget.NewLabel("O(∩_∩)O哈哈~")
	}

	// Update table
	update := func(id widget.TableCellID, cell fyne.CanvasObject) {
		label := cell.(*widget.Label)
		label.SetText(chats[id.Row][id.Col])
		if withTruncation {
			// auto truncation method (change 'withTruncation' to 'false' to disable it)
			label.Truncation = fyne.TextTruncateEllipsis
		}
	}

	// Create table
	table := widget.NewTableWithHeaders(length, create, update)

	// Adjust height/width
	for i, v := range chats {

		for col, text := range v {
			switch col {
			case 0:
				if len(text) > 25 {
					table.SetRowHeight(i, 50)
				}
			case 1:
				// do something
			case 2:
				// do...
			}

			if !withTruncation {
				// manual cell width sizing method (change 'withTruncation' to 'true' to disable it)
				width := fyne.MeasureText(text, theme.TextSize(), fyne.TextStyle{}).Width
				table.SetColumnWidth(col, width+theme.Padding()*5)
			}
		}
	}

	return table
}

@MidnightSong
Copy link
Author

Hi,

Personally, I don't use 'table.SetRowHeight' inside the 'update' function because it seems logical that an issue will occur, given that the row size increases in height, it will trigger a table update again and this will create an infinite loop that will ultimately result in a stack overflow (This is what I believe, but I might be mistaken).

So, to keep it simple, you put 'table.SetRowHeight' right after creating your table, something like this:

        var table *widget.Table
       
	// ...

	// ...

	table = widget.NewTable(length, create, update)

	for i := range items.EnvItems {
		table.SetRowHeight(i, rowHeight)
	}

	return table

And it works perfectly.

Here's a complete example:

package main

import (
	"fyne.io/fyne/v2"
	"fyne.io/fyne/v2/app"
	"fyne.io/fyne/v2/container"
	"fyne.io/fyne/v2/theme"
	"fyne.io/fyne/v2/widget"
)

var (
	// Dummy data
	chats = [][]string{
		{"title0", "content0", "author0"},
		{"title1", "content1", "author1"},
		{"title2", "content2", "author2"},
		{"title3verylongwithlotofwords", "content3", "author3"},
		{"title4", "content4", "author4"},
		{"title5", "content5", "author5"},
		{"title6verylongwithlotofwords", "content6", "author6"},
		{"title7verylongwithlotofwords", "content7", "author7"},
		{"title8", "content8", "author8"},
		{"title9verylongwithlotofwords", "content9", "author9"},
	}

	// Set to false to disable text truncation.
	// It will use manual width sizing instead.
	withTruncation bool = true
)

func main() {
	mainApp := app.NewWithID("a.b.c.d")
	mainWin := mainApp.NewWindow("test table")

	table := openedDialogs()

	mainWin.SetContent(container.NewStack(table))
	mainWin.CenterOnScreen()
	mainWin.Resize(fyne.NewSize(800, 400))

	mainWin.ShowAndRun()
}

func openedDialogs() *widget.Table {
	// Get length
	length := func() (int, int) { return len(chats), 3 }

	// Create label
	create := func() fyne.CanvasObject {
		return widget.NewLabel("O(∩_∩)O哈哈~")
	}

	// Update table
	update := func(id widget.TableCellID, cell fyne.CanvasObject) {
		label := cell.(*widget.Label)
		label.SetText(chats[id.Row][id.Col])
		if withTruncation {
			// auto truncation method (change 'withTruncation' to 'false' to disable it)
			label.Truncation = fyne.TextTruncateEllipsis
		}
	}

	// Create table
	table := widget.NewTableWithHeaders(length, create, update)

	// Adjust height/width
	for i, v := range chats {

		for col, text := range v {
			switch col {
			case 0:
				if len(text) > 25 {
					table.SetRowHeight(i, 50)
				}
			case 1:
				// do something
			case 2:
				// do...
			}

			if !withTruncation {
				// manual cell width sizing method (change 'withTruncation' to 'true' to disable it)
				width := fyne.MeasureText(text, theme.TextSize(), fyne.TextStyle{}).Width
				table.SetColumnWidth(col, width+theme.Padding()*5)
			}
		}
	}

	return table
}

Thank you for your answer. This is indeed a temporary solution that can be used

@MidnightSong
Copy link
Author

Is it possible to please provide a code sample that actually replicates this issue? The snippet included does not compile.

I have modified the example code. Could you please review it again

andydotxyz added a commit to andydotxyz/fyne that referenced this issue Jul 19, 2024
@andydotxyz
Copy link
Member

Thanks, I can now replicate the issue. PR open

@dweymouth dweymouth changed the title stack overflow stack overflow when calling SetRowHeight in table UpdateCell callback Jul 19, 2024
@dweymouth dweymouth added bug Something isn't working and removed unverified A bug that has been reported but not verified labels Jul 19, 2024
@dweymouth dweymouth added this to the E fixes (v2.5.x) milestone Jul 19, 2024
@JanVaya
Copy link

JanVaya commented Jul 25, 2024

I had the same problem but could not use the proposed solution. I wordwrap Text in Cells and thus the columnHeight changes, if the contents or the columnWidth changes. Therefore I used SetRowHeight in the UpdateCell call. If fixed the initial problem with a simple variable based saveguard, preventing that I use SetRowHeight again during the Refresh, that it caused:
label.SetText(currentRSListe.Liste[id.Row].Beschreibung)
newHeight := label.MinSize().Height
if abs(newHeight-cell.Size().Height) > 0.1*newHeight {
if gateKeeper == 0 {
gateKeeper = 1
table.SetRowHeight(id.Row, newHeight)
gateKeeper = 0
}
}
But even with this fix it crashes, because SetRowHeight is changing the internal map rowHeights and this is also accessed heavily during other ongoing Refreshes and after some scrolling of the table the program will panic with concurrent map write...

I have fixed this with the introduction of a Mutex to guard all accesses the columnWidths and rowHeights maps in table.go and now it all runs smoothly. I tried to make a fork of fyne and made the changes to table.go there but failed to execute the tests as go test ..... FAILs in the canvas.go which I didn't touch :-(
--- FAIL: TestGlCanvas_ChildMinSizeChangesInDifferentScrollAffectAncestorsUpToScroll (0.33s)
canvas_test.go:117:
Error Trace: D:/Go/Projekte/fyne/internal/driver/glfw/canvas_test.go:117
Error: Not equal:
expected: fyne.Size{Width:76, Height:40}
actual : fyne.Size{Width:120, Height:40}

                            Diff:
                            --- Expected
                            +++ Actual
                            @@ -1,3 +1,3 @@
                             (fyne.Size) {
                            - Width: (float32) 76,
                            + Width: (float32) 120,
                              Height: (float32) 40
            Test:           TestGlCanvas_ChildMinSizeChangesInDifferentScrollAffectAncestorsUpToScroll

2024/07/25 21:52:08 Fyne error: Failed to focus object which is not part of the canvas’ content, menu or overlays.
2024/07/25 21:52:08 At: D:/Go/Projekte/fyne/internal/driver/common/canvas.go:182

Hope this helps anyone.

@andydotxyz
Copy link
Member

Fixed on develop and will get picked into v2.5.1

@janmz
Copy link

janmz commented Aug 8, 2024

Thank You!!

andydotxyz added a commit that referenced this issue Aug 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants