Fix highlighting crash when line index is outdated

[omarelladen]

A random crash happened to me:

Micro encountered an error: runtime.boundsError runtime error: index out of range [197] with length 197
runtime/panic.go:115 (0x55a67689b1f4)
github.com/zyedidia/micro/internal/buffer/line_array.go:394 (0x55a676cf4c32)
github.com/zyedidia/micro/internal/buffer/buffer.go:1423 (0x55a676d12c88)
github.com/zyedidia/micro/internal/display/bufwindow.go:494 (0x55a676d12c3f)
github.com/zyedidia/micro/internal/display/bufwindow.go:756 (0x55a676d11833)
github.com/zyedidia/micro/internal/display/bufwindow.go:827 (0x55a676d14319)
github.com/zyedidia/micro/cmd/micro/micro.go:413 (0x55a676d73db5)
github.com/zyedidia/micro/cmd/micro/micro.go:400 (0x55a676d73b30)
internal/runtime/atomic/types.go:194 (0x55a6768a053d)
runtime/asm_amd64.s:1700 (0x55a6768dc0c1)

If you can reproduce this error, please report it at https://github.com/zyedidia/micro/issues

Since I was using version 2.0.14-1+b6 from Debian, the problem is in this line:

micro/internal/buffer/line_array.go

Line 389 in 77de728

if la.lines[lineN].search == nil {

(check https://sources.debian.org/src/micro/2.0.14-1/internal/buffer/line_array.go#L394)

I could not reproduce the bug, but it seems to me that the highlighter was trying to access a line that was just deleted, using an outdated Y position of the window. So this proposed fix is simple, but the concurrent access problem can be deeper.

[Neko-Box-Coder]

@dmaluka @JoeKar

I just got this as well and lost (unsaved) data (although nothing too important) because of this.

Micro encountered an error: runtime.boundsError runtime error: index out of range [289] with length 280
runtime/panic.go:236 (0x44eb97)
runtime/asm_amd64.s:2129 (0x491368)
github.com/micro-editor/micro/v2/internal/buffer/line_array.go:389 (0x893005)
github.com/micro-editor/micro/v2/internal/buffer/buffer.go:1651 (0x8b242c)
github.com/micro-editor/micro/v2/internal/display/bufwindow.go:626 (0x8b23e8)
github.com/micro-editor/micro/v2/internal/display/bufwindow.go:848 (0x8b1a39)
github.com/micro-editor/micro/v2/internal/display/bufwindow.go:919 (0x8b3b99)
github.com/micro-editor/micro/v2/cmd/micro/micro.go:512 (0x913235)
github.com/micro-editor/micro/v2/cmd/micro/micro.go:499 (0x9130c5)
internal/runtime/atomic/types.go:194 (0x453c75)
runtime/asm_amd64.s:1771 (0x490f01)

Maybe we should merge this since it is a crash, not to mention plugins can call this directly with out of bound Loc and crash it as well.

[JoeKar]

I saw this PR already, but had the same feeling as @omarelladen with...

[...], but the concurrent access problem can be deeper.

...that it will cover/hide the real root cause of asynchronous access or insufficient access order.
Whilst this may be a quick solution, it carries the risk of losing sight of the actual cause.
We should inspect the bloc and la.lines a bit deeper.

[Neko-Box-Coder]

@JoeKar

Just to make sure I am on the same page, correct me if I am wrong, the crash is in

micro/internal/display/bufwindow.go

Line 383 in 77de728

func (w *BufWindow) displayBuffer() {

which is called from

micro/cmd/micro/micro.go

Line 504 in 22930a5

func DoEvent() {

...that it will cover/hide the real root cause of asynchronous access or insufficient access order.

What is running asynchronously/concurrently when displayBuffer() is running?

Otherwise, somehow bloc in (displayBuffer()) got an invalid value, as you suggested.

[dmaluka]

What is running asynchronously/concurrently when displayBuffer() is running?

A couple of things are running concurrently with it (e.g. syntax highlighter, background backups) but none of them are supposed to modify the buffer. It is only supposed to be modified by the main goroutine.

[Neko-Box-Coder]

I took a look at all the places that have go func() and couldn't really see anything immediately obvious that modifies the buffer.

I agree the current fix will cover/obfuscate the issue, but at the same time I feel like we should be doing something better than straight up crashing atm. Unless someone can have another go to see see if there's anything obvious that I missed (which is like not unlikely, golang is not my main lang afterall)

If not, maybe we can:

1. Catch this crash, since we know exactly where it crashes
2. Save all the buffers (including unnamed ones maybe?), to backup dir (Unless we already have this mechanism of saving automatically when crashing?...).
3. Print all the (local/global) variables (such as bloc, bufer status, etc...) that might help triage this issue.
4. Panic/Crash the micro to avoid continuing in a corrupted state? (Or recover, although I don't know how exactly...)

How does this sound?

[Edit]: Ideally I do want to merge this... at some point since plugins can call this function and crash micro by passing an invalid arg.

[JoeKar]

StartLine is modified by scroll events and relocations. But I wasn't able to remove lines, while the active cursor is kept at the last line of the buffer. The only thing that "worked" for me was the textfilter, but this only revealed, that it doesn't even relocate at the end...while it should.

@Neko-Box-Coder
Can you remember any special use case, while you reproduced this by accident?

Print all the (local/global) variables (such as bloc, bufer status, etc...) [...]

The bloc was printed implicit:

Micro encountered an error: runtime.boundsError runtime error: index out of range [289] with length 280

It was off by 9.