Fyne: widget.NewMultiLineEntry panics after several lines of input

Created on 21 Mar 2019  ·  21Comments  ·  Source: fyne-io/fyne 
package main

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

func main() {
    app := app.New()
    w := app.NewWindow("Hello")
    w.SetContent(widget.NewVBox(
        widget.NewMultiLineEntry(),
    ))
    w.ShowAndRun()
}

Click to focus on the text field, type text and hit enter often. It crashes typically within 10 lines of input.

panic: runtime error: index out of range

goroutine 1 [running, locked to thread]:
fyne.io/fyne/widget.(*textProvider).row(...)
    /home/tv/go/pkg/mod/fyne.io/[email protected]/widget/text.go:171
fyne.io/fyne/widget.(*textRenderer).lineSize(0xc00009a2c0, 0x3, 0x1, 0xc00009a2c0, 0xc00009a180)
    /home/tv/go/pkg/mod/fyne.io/[email protected]/widget/text.go:279 +0x205
fyne.io/fyne/widget.(*entryRenderer).moveCursor(0xc00009a180)
    /home/tv/go/pkg/mod/fyne.io/[email protected]/widget/entry.go:45 +0x7f
fyne.io/fyne/widget.(*Entry).TypedRune(0xc000128090, 0xc000000066)
    /home/tv/go/pkg/mod/fyne.io/[email protected]/widget/entry.go:236 +0x145
fyne.io/fyne/driver/gl.(*window).charModInput(0xc0000fa100, 0xc0000fa080, 0x66, 0x0)
    /home/tv/go/pkg/mod/fyne.io/[email protected]/driver/gl/window.go:782 +0x7f
github.com/go-gl/glfw/v3.2/glfw.goCharModsCB(0x1584da0, 0x66)
    /home/tv/go/pkg/mod/github.com/go-gl/[email protected]/v3.2/glfw/input.go:300 +0x5d
github.com/go-gl/glfw/v3.2/glfw._cgoexpwrap_7d737b30959b_goCharModsCB(0x1584da0, 0x66)
    _cgo_gotypes.go:2176 +0x3b
github.com/go-gl/glfw/v3.2/glfw._Cfunc_glfwPollEvents()
    _cgo_gotypes.go:1212 +0x41
github.com/go-gl/glfw/v3.2/glfw.PollEvents()
    /home/tv/go/pkg/mod/github.com/go-gl/[email protected]/v3.2/glfw/window.go:777 +0x22
fyne.io/fyne/driver/gl.(*gLDriver).runGL(0xc0000a2780)
    /home/tv/go/pkg/mod/fyne.io/[email protected]/driver/gl/loop.go:77 +0x1d5
fyne.io/fyne/driver/gl.(*gLDriver).Run(0xc0000a2780)
    /home/tv/go/pkg/mod/fyne.io/[email protected]/driver/gl/driver.go:55 +0x43
fyne.io/fyne/driver/gl.(*window).ShowAndRun(0xc0000fa100)
    /home/tv/go/pkg/mod/fyne.io/[email protected]/driver/gl/window.go:288 +0x58
main.main()
    /home/tv/src/2019/learn-fyne/main.go:16 +0x15e
bug
Source
picture tv42
👍1

Most helpful comment

@andydotxyz

One thing to consider is that a race does not necessarily mean a crash

It means something much much worse than a crash.
It means that a pack of _heisenbugs_ is lurking along the way.

picture ohir on 26 Mar 2019
👍6

All 21 comments

panic: runtime error: index out of range

goroutine 133 [running]:
fyne.io/fyne/widget.(*textRenderer).lineSize(0xc00009a2c0, 0x0, 0x7, 0xc00009a2c0, 0xc00009a180)
    /home/tv/go/pkg/mod/fyne.io/[email protected]/widget/text.go:284 +0x1f7
fyne.io/fyne/widget.(*entryRenderer).moveCursor(0xc00009a180)
    /home/tv/go/pkg/mod/fyne.io/[email protected]/widget/entry.go:45 +0x7f
fyne.io/fyne/widget.(*Entry).TypedKey(0xc000128090, 0xc000182890)
    /home/tv/go/pkg/mod/fyne.io/[email protected]/widget/entry.go:336 +0x1d0
created by fyne.io/fyne/driver/gl.(*window).keyPressed
    /home/tv/go/pkg/mod/fyne.io/[email protected]/driver/gl/window.go:744 +0x29b
tv42 picture tv42 on 21 Mar 2019
👍1

I cannot replicate locally - what system information can you share to help narrow this down?

andydotxyz picture andydotxyz on 21 Mar 2019

Ubuntu 18.04. I'll try to get something more reproducible with a fresh VM or something like that.

tv42 picture tv42 on 23 Mar 2019

That would be really helpful thanks

andydotxyz picture andydotxyz on 23 Mar 2019

I was able to reproduce on my machine, a debian 9 kde desktop environment with golang 1.11.5 .

I think the problem is a slight race condition in textRenderer.Refresh(), when r.texts is getting re-initialized and then re-populated, but if multiline is expecting a row beyond the first row, it gets an invalid index. I put some mutexes around problem points and will make a PR, @tv42 could you help to check that the issue goes away for you too?

paulhovey picture paulhovey on 24 Mar 2019

It seems this is very much not the only race in fyne. Build a demo app with -race and you'll see a constant stream of errors.

tv42 picture tv42 on 25 Mar 2019

I can still reproduce the multiline text entry panic with pr #179. It takes a little longer to trigger a panic, but I just got this:

panic: runtime error: index out of range

goroutine 318 [running]:
fyne.io/fyne/widget.(*textProvider).row(0xc0000fc180, 0x6d, 0x0, 0x0, 0x0)
    /home/tv/src/others/fyne/widget/text.go:179 +0x131
fyne.io/fyne/widget.(*textRenderer).lineSize(0xc0000a4190, 0x0, 0x6d, 0xc0000a4190, 0xc0000a2180)
    /home/tv/src/others/fyne/widget/text.go:296 +0x51
fyne.io/fyne/widget.(*entryRenderer).moveCursor(0xc0000a2180)
    /home/tv/src/others/fyne/widget/entry.go:45 +0x7f
fyne.io/fyne/widget.(*Entry).TypedKey(0xc00012a090, 0xc000184590)
    /home/tv/src/others/fyne/widget/entry.go:335 +0x180
created by fyne.io/fyne/driver/gl.(*window).keyPressed
    /home/tv/src/others/fyne/driver/gl/window.go:754 +0x29b

Given the number of races exposed by -race, it's perfectly plausible this is now a different race causing a crash. The core of fyne needs to be redone with an understanding of concurrency, not just patched until it "works for me".

tv42 picture tv42 on 25 Mar 2019

Though you may have a point, @tv42, I'm sure it could have been approached in a more constructive manner.

There are many areas in the core that are subject to race conditions, the most likely noticeable ones are around text handling as the library we use is not thread safe.

We are keen to make sure that the thread complexities do not seep out of the core API and that is a difficult balance to manage. One thing to consider is that a race does not necessarily mean a crash - we are tackling them in order. If you would like to contribute something to help in this area we would be most grateful.

andydotxyz picture andydotxyz on 25 Mar 2019

@andydotxyz

One thing to consider is that a race does not necessarily mean a crash

It means something much much worse than a crash.
It means that a pack of _heisenbugs_ is lurking along the way.

ohir picture ohir on 26 Mar 2019
👍6

Two thoughts:

  1. There's no such thing as a benign race.
  2. Fixing concurrency issues after the fact, and not at the design phase, is the hardest possible route.
tv42 picture tv42 on 26 Mar 2019

Yes those points are probably true, thanks.
We have a lot of work to balance the ease of use of a GUI api in Go (including that GUI updates should work from goroutines) and the safety that should be enforced by a toolkit. I think that this should be possible by enforcing the atomicity of underlying data in each of the widgets/canvas objects - after developing this pattern we should be able to remove some potential workaround mutexes.

andydotxyz picture andydotxyz on 26 Mar 2019

See also - https://github.com/fyne-io/fyne/issues/189

There are several places in the text widget with the same condition. Cause is during the Refresh() the []texts is being reset to a new blank slice, whilst other goroutines may be iterating over the existing slice.

Fix is an easy one, will post a PR tonight when I get home. Fix is internal only, and does not impact the API. Its not a design blunder that caused the problem, its just something that was overlooked in code review.

Slice iteration concurrency issues are quite easy to overlook, because the data races only occur in really small time windows ... and Go coders tend to assume that slices are safe, because they are safer than maps .... so they are also quite common !

If you range over slice S, and the value of S itself changes half way through the iteration, then the range will continue to iterate over the original value of S . (dump the compiler output to asm to prove this possibly unexpected behaviour)

Dont stress - Ive seen exactly the same bug in big heavy volume production environments, built from scratch to "do concurrency right" ... and which get reviewed and QA'd to the Nth degree before the code goes anywhere near prod.

Yet they still crash on the rare weekend right in middle of a busy transaction event, with weird "invalid memory access" errors that cant be reproduced in testing.

Not a problem :)

steveoc64 picture steveoc64 on 27 Mar 2019
😄1 👎1

There is an underlying issue with how we handle the text library that is about to be merged.

194 is up for review just now - I don't know if you can test locally but it should be part of the repo soon.

andydotxyz picture andydotxyz on 29 Mar 2019

I updated #179 also after doing more testing with -race . Last time I ran it, I was unable to reproduce the bug.

paulhovey picture paulhovey on 29 Mar 2019

I can no longer reproduce this issue either

andydotxyz picture andydotxyz on 7 Apr 2019

Built a simple app with -race using 2cc8dfa90d47, errors immediately:

==================
WARNING: DATA RACE
Read at 0x00c00008a6d0 by main goroutine:
  fyne.io/fyne/app.(*settings).AddChangeListener()
      /home/tv/go/pkg/mod/fyne.io/[email protected]/app/settings.go:21 +0x47
  fyne.io/fyne/driver/gl.(*gLDriver).runGL()
      /home/tv/go/pkg/mod/fyne.io/[email protected]/driver/gl/loop.go:61 +0x123
  fyne.io/fyne/driver/gl.(*gLDriver).Run()
      /home/tv/go/pkg/mod/fyne.io/[email protected]/driver/gl/driver.go:58 +0x50
  fyne.io/fyne/driver/gl.(*window).ShowAndRun()
      /home/tv/go/pkg/mod/fyne.io/[email protected]/driver/gl/window.go:303 +0x75
  main.main()
      /home/tv/src/2019/learn-fyne/main.go:26 +0x3ce

Previous write at 0x00c00008a6d0 by goroutine 10:
  fyne.io/fyne/app.(*settings).AddChangeListener()
      /home/tv/go/pkg/mod/fyne.io/[email protected]/app/settings.go:21 +0xb6
  fyne.io/fyne/driver/gl.svgCacheMonitorTheme()
      /home/tv/go/pkg/mod/fyne.io/[email protected]/driver/gl/svg_cache.go:89 +0x98

Goroutine 10 (finished) created at:
  fyne.io/fyne/driver/gl.(*gLDriver).Run()
      /home/tv/go/pkg/mod/fyne.io/[email protected]/driver/gl/driver.go:57 +0x42
  fyne.io/fyne/driver/gl.(*window).ShowAndRun()
      /home/tv/go/pkg/mod/fyne.io/[email protected]/driver/gl/window.go:303 +0x75
  main.main()
      /home/tv/src/2019/learn-fyne/main.go:26 +0x3ce
==================
==================
WARNING: DATA RACE
Read at 0x00c000302008 by main goroutine:
  runtime.growslice()
      /home/tv/sdk/go1.12.1/src/runtime/slice.go:76 +0x0
  fyne.io/fyne/app.(*settings).AddChangeListener()
      /home/tv/go/pkg/mod/fyne.io/[email protected]/app/settings.go:21 +0x13e
  fyne.io/fyne/driver/gl.(*gLDriver).runGL()
      /home/tv/go/pkg/mod/fyne.io/[email protected]/driver/gl/loop.go:61 +0x123
  fyne.io/fyne/driver/gl.(*gLDriver).Run()
      /home/tv/go/pkg/mod/fyne.io/[email protected]/driver/gl/driver.go:58 +0x50
  fyne.io/fyne/driver/gl.(*window).ShowAndRun()
      /home/tv/go/pkg/mod/fyne.io/[email protected]/driver/gl/window.go:303 +0x75
  main.main()
      /home/tv/src/2019/learn-fyne/main.go:26 +0x3ce

Previous write at 0x00c000302008 by goroutine 10:
  fyne.io/fyne/app.(*settings).AddChangeListener()
      /home/tv/go/pkg/mod/fyne.io/[email protected]/app/settings.go:21 +0x89
  fyne.io/fyne/driver/gl.svgCacheMonitorTheme()
      /home/tv/go/pkg/mod/fyne.io/[email protected]/driver/gl/svg_cache.go:89 +0x98

Goroutine 10 (finished) created at:
  fyne.io/fyne/driver/gl.(*gLDriver).Run()
      /home/tv/go/pkg/mod/fyne.io/[email protected]/driver/gl/driver.go:57 +0x42
  fyne.io/fyne/driver/gl.(*window).ShowAndRun()
      /home/tv/go/pkg/mod/fyne.io/[email protected]/driver/gl/window.go:303 +0x75
  main.main()
      /home/tv/src/2019/learn-fyne/main.go:26 +0x3ce
==================
tv42 picture tv42 on 8 Apr 2019

That seems like a separate issue to the one in the title. Is the application malfunctioning without “-race”?

There are still some race conditions, yes, we are tackling this in order of impact I think.

andydotxyz picture andydotxyz on 8 Apr 2019

Sure, it's not exactly the same race. If you want to pretend things work while -race still complains, go right ahead.

tv42 picture tv42 on 9 Apr 2019

If you would like to open a new ticket to help us work to resolve the settings issue that would be very welcome.
If you are not interested in a constructive approach then we will manage without your input.

andydotxyz picture andydotxyz on 9 Apr 2019
❤2 👎2

It seems like you are not "managing".

4ad picture 4ad on 9 Apr 2019

Hi @4ad - I am not sure what you mean by this message. If there is some way that this toolkit is failing for you then please file an issue so it can be resolved.

andydotxyz picture andydotxyz on 9 Apr 2019
👍1
Was this page helpful?
0 / 5 - 0 ratings

Related issues

Geo25rey picture Geo25rey  ·  7Comments
Jacalz picture Jacalz  ·  4Comments
develar picture develar  ·  6Comments
andydotxyz picture andydotxyz  ·  7Comments
semyon-dev picture semyon-dev  ·  4Comments