nelsam/vidar

Incorrect removing when "massive" backspacing.

Closed this issue · 6 comments

Description

Incorrect removing when "massive" backspacing.

Steps to reproduce

  1. Press key backspace in middle of text
  2. Waiting
  3. Symbols before caret will be removed

Expected behavior

Removing only previous symbols.

Actual behavior

Symbols deleting in both direction

Stack trace

Guess the reason is in the data race, log from build with -race flag:

==================
WARNING: DATA RACE
Read at 0x00c04220e9e0 by goroutine 32:
  github.com/nelsam/gxui.(*TextBoxController).Text()
      E:/gopath/src/github.com/nelsam/gxui/textbox_controller.go:228 +0x47
  github.com/nelsam/gxui/mixins.(*TextBox).Text()
      E:/gopath/src/github.com/nelsam/gxui/mixins/textbox.go:234 +0x5b
  github.com/nelsam/vidar/editor.(*CodeEditor).Text()
      <autogenerated>:1 +0x4a
  github.com/nelsam/vidar/plugin/gosyntax.(*Highlight).TextChanged()
      E:/gopath/src/github.com/nelsam/vidar/plugin/gosyntax/highlight.go:77 +0x63
  github.com/nelsam/vidar/command/input.(*ctxHookReader).textChanged.func1()
      E:/gopath/src/github.com/nelsam/vidar/command/input/context_change_hook.go :78 +0x135

Previous write at 0x00c04220e9e0 by goroutine 6:
  [failed to restore the stack]

Goroutine 32 (running) created at:
  github.com/nelsam/vidar/command/input.(*ctxHookReader).textChanged()
      E:/gopath/src/github.com/nelsam/vidar/command/input/context_change_hook.go
:71 +0x42a
  github.com/nelsam/vidar/command/input.(*Handler).textEdited()
      E:/gopath/src/github.com/nelsam/vidar/command/input/handler.go:248 +0x1db
  github.com/nelsam/vidar/command/input.(*Handler).Apply.func2()
      E:/gopath/src/github.com/nelsam/vidar/command/input/handler.go:145 +0xf2
  github.com/nelsam/gxui/drivers/gl.(*driver).CallSync()
      E:/gopath/src/github.com/nelsam/gxui/drivers/gl/driver.go:129 +0x1ac
  github.com/nelsam/vidar/command/input.(*Handler).Apply()
      E:/gopath/src/github.com/nelsam/vidar/command/input/handler.go:143 +0xa87
  github.com/nelsam/vidar/command/input.(*Handler).HandleEvent()
      E:/gopath/src/github.com/nelsam/vidar/command/input/handler.go:210 +0xea6
  github.com/nelsam/vidar/commander.(*Commander).KeyPress()
      E:/gopath/src/github.com/nelsam/vidar/commander/commander.go:288 +0x4ac
  github.com/nelsam/gxui.(*KeyboardController).keyPress()
      E:/gopath/src/github.com/nelsam/gxui/keyboard_controller.go:42 +0xd9
  github.com/nelsam/gxui.(*KeyboardController).(github.com/nelsam/gxui.keyPress)
-fm()
      E:/gopath/src/github.com/nelsam/gxui/keyboard_controller.go:17 +0x5c
  runtime.call32()
      E:/go/src/runtime/asm_amd64.s:509 +0x41
  reflect.Value.Call()
      E:/go/src/reflect/value.go:302 +0xc7
  github.com/nelsam/gxui.(*EventBase).InvokeListeners()
      E:/gopath/src/github.com/nelsam/gxui/event_base.go:140 +0x40d
  github.com/nelsam/gxui.(*EventBase).Fire()
      E:/gopath/src/github.com/nelsam/gxui/event_base.go:190 +0x8d
  github.com/nelsam/gxui.(*SimpleEvent).Fire()
      <autogenerated>:1 +0x68
  github.com/nelsam/gxui/mixins.(*Window).setViewport.func11()
      E:/gopath/src/github.com/nelsam/gxui/mixins/window.go:378 +0x127
  runtime.call32()
      E:/go/src/runtime/asm_amd64.s:509 +0x41
  reflect.Value.Call()
      E:/go/src/reflect/value.go:302 +0xc7
  github.com/nelsam/gxui.(*EventBase).InvokeListeners()
      E:/gopath/src/github.com/nelsam/gxui/event_base.go:140 +0x40d
  github.com/nelsam/gxui.(*ChanneledEvent).Fire.func1()
      E:/gopath/src/github.com/nelsam/gxui/channeled_event.go:40 +0x97
  github.com/nelsam/gxui/drivers/gl.(*driver).applicationLoop()
      E:/gopath/src/github.com/nelsam/gxui/drivers/gl/driver.go:111 +0x38

  Goroutine 6 (running) created at:
  github.com/nelsam/gxui/drivers/gl.StartDriver()
      E:/gopath/src/github.com/nelsam/gxui/drivers/gl/driver.go:58 +0x5f1
  main.init.0.func1()
      E:/gopath/src/github.com/nelsam/vidar/main.go:51 +0x86
  github.com/spf13/cobra.(*Command).execute()
      E:/gopath/src/github.com/spf13/cobra/command.go:760 +0x88d
  github.com/spf13/cobra.(*Command).ExecuteC()
      E:/gopath/src/github.com/spf13/cobra/command.go:846 +0x4e2
  github.com/spf13/cobra.(*Command).Execute()
      E:/gopath/src/github.com/spf13/cobra/command.go:794 +0x3f
  main.main()
      E:/gopath/src/github.com/nelsam/vidar/main.go:57 +0x51
==================
==================
WARNING: DATA RACE
Read at 0x00c0438cf8b4 by goroutine 32:
  github.com/nelsam/gxui.RuneArrayToString()
      E:/gopath/src/github.com/nelsam/gxui/utils.go:232 +0x13e
  github.com/nelsam/gxui.(*TextBoxController).Text()
      E:/gopath/src/github.com/nelsam/gxui/textbox_controller.go:228 +0x6b
  github.com/nelsam/gxui/mixins.(*TextBox).Text()
      E:/gopath/src/github.com/nelsam/gxui/mixins/textbox.go:234 +0x5b
  github.com/nelsam/vidar/editor.(*CodeEditor).Text()
      <autogenerated>:1 +0x4a
  github.com/nelsam/vidar/plugin/gosyntax.(*Highlight).TextChanged()
      E:/gopath/src/github.com/nelsam/vidar/plugin/gosyntax/highlight.go:77 +0x6
3
  github.com/nelsam/vidar/command/input.(*ctxHookReader).textChanged.func1()
      E:/gopath/src/github.com/nelsam/vidar/command/input/context_change_hook.go
:78 +0x135

Previous write at 0x00c0438cf8b4 by goroutine 6:
  [failed to restore the stack]

 Goroutine 32 (running) created at:
  github.com/nelsam/vidar/command/input.(*ctxHookReader).textChanged()
      E:/gopath/src/github.com/nelsam/vidar/command/input/context_change_hook.go
:71 +0x42a
  github.com/nelsam/vidar/command/input.(*Handler).textEdited()
      E:/gopath/src/github.com/nelsam/vidar/command/input/handler.go:248 +0x1db
  github.com/nelsam/vidar/command/input.(*Handler).Apply.func2()
      E:/gopath/src/github.com/nelsam/vidar/command/input/handler.go:145 +0xf2
  github.com/nelsam/gxui/drivers/gl.(*driver).CallSync()
      E:/gopath/src/github.com/nelsam/gxui/drivers/gl/driver.go:129 +0x1ac
  github.com/nelsam/vidar/command/input.(*Handler).Apply()
      E:/gopath/src/github.com/nelsam/vidar/command/input/handler.go:143 +0xa87
  github.com/nelsam/vidar/command/input.(*Handler).HandleEvent()
      E:/gopath/src/github.com/nelsam/vidar/command/input/handler.go:210 +0xea6
  github.com/nelsam/vidar/commander.(*Commander).KeyPress()
      E:/gopath/src/github.com/nelsam/vidar/commander/commander.go:288 +0x4ac
  github.com/nelsam/gxui.(*KeyboardController).keyPress()
      E:/gopath/src/github.com/nelsam/gxui/keyboard_controller.go:42 +0xd9
  github.com/nelsam/gxui.(*KeyboardController).(github.com/nelsam/gxui.keyPress)
-fm()
      E:/gopath/src/github.com/nelsam/gxui/keyboard_controller.go:17 +0x5c
  runtime.call32()
      E:/go/src/runtime/asm_amd64.s:509 +0x41
  reflect.Value.Call()
      E:/go/src/reflect/value.go:302 +0xc7
  github.com/nelsam/gxui.(*EventBase).InvokeListeners()
      E:/gopath/src/github.com/nelsam/gxui/event_base.go:140 +0x40d
  github.com/nelsam/gxui.(*EventBase).Fire()
      E:/gopath/src/github.com/nelsam/gxui/event_base.go:190 +0x8d
  github.com/nelsam/gxui.(*SimpleEvent).Fire()
      <autogenerated>:1 +0x68
  github.com/nelsam/gxui/mixins.(*Window).setViewport.func11()
      E:/gopath/src/github.com/nelsam/gxui/mixins/window.go:378 +0x127
  runtime.call32()
      E:/go/src/runtime/asm_amd64.s:509 +0x41
  reflect.Value.Call()
      E:/go/src/reflect/value.go:302 +0xc7
  github.com/nelsam/gxui.(*EventBase).InvokeListeners()
      E:/gopath/src/github.com/nelsam/gxui/event_base.go:140 +0x40d
  github.com/nelsam/gxui.(*ChanneledEvent).Fire.func1()
      E:/gopath/src/github.com/nelsam/gxui/channeled_event.go:40 +0x97
  github.com/nelsam/gxui/drivers/gl.(*driver).applicationLoop()
      E:/gopath/src/github.com/nelsam/gxui/drivers/gl/driver.go:111 +0x38
  Goroutine 6 (running) created at:
  github.com/nelsam/gxui/drivers/gl.StartDriver()
      E:/gopath/src/github.com/nelsam/gxui/drivers/gl/driver.go:58 +0x5f1
  main.init.0.func1()
      E:/gopath/src/github.com/nelsam/vidar/main.go:51 +0x86
  github.com/spf13/cobra.(*Command).execute()
      E:/gopath/src/github.com/spf13/cobra/command.go:760 +0x88d
  github.com/spf13/cobra.(*Command).ExecuteC()
      E:/gopath/src/github.com/spf13/cobra/command.go:846 +0x4e2
  github.com/spf13/cobra.(*Command).Execute()
      E:/gopath/src/github.com/spf13/cobra/command.go:794 +0x3f
  main.main()
      E:/gopath/src/github.com/nelsam/vidar/main.go:57 +0x51
==================
Found 2 data race(s)

This behaviour(data race) related not only for backspace action but also can be reproducing with paste operation.

Yep, this is a problem where editor.Text() is being called in a non-UI goroutine while editor.SetText() is being called in the UI goroutine. I've got some ideas in my head about how to solve this, but I would like to do some benchmarks. Anything relating to user input should stay as fast as possible, so I don't want to implement a solution without exploring a little.

I've added a preliminary fix in #128 and nelsam/gxui#8. I have to try to run the editor with -race turned on for a while before I'll be confident with the fix, but it seems to work.

nelsam/gxui#8 and #128 were merged. It seemed like it was working fine. I made some attempts to trigger the data race again and couldn't, so I think it's working at least a bit cleaner.

I was having a hard time triggering it on my computer even before the fixes, though, so let me know if you still see problems.

Yes, after the patch it works as should only deprecation warning trigger for copy paste here.

Fixed the deprecation warnings. I'm going to be removing those pretty soon. I just wanted a short transition period to make sure there wasn't too much overlap where vidar couldn't be built.