walles / moar

Moar is a pager. It's designed to just do the right thing without any configuration.
Other
676 stars 19 forks source link

A crash #166

Closed postsolar closed 12 months ago

postsolar commented 12 months ago

Opened a man page, made a search, hit n a couple times, scrolled up with ↑ for a while, and at some point it crashed. Can't reproduce.

Here's the log:

Crash log ``` Please post the following report at , or e-mail it to johan.walles@gmail.com. Version: 1.18.4 LANG : en_US.UTF-8 TERM : tmux-256color GOOS : linux GOARCH : amd64 Compiler: gc NumCPU : 4 panic: lineNumberString <1000 > longer than numberPrefixLength 4 [recovered] panic: lineNumberString <1000 > longer than numberPrefixLength 4 [recovered] panic: lineNumberString <1000 > longer than numberPrefixLength 4 goroutine 1 [running]: main.main.func1() github.com/walles/moar/moar.go:333 +0x155 panic({0x55bd917858e0?, 0xc000322190?}) runtime/panic.go:914 +0x21f main.startPaging.func1() github.com/walles/moar/moar.go:538 +0xb0 panic({0x55bd917858e0?, 0xc000322190?}) runtime/panic.go:914 +0x21f github.com/walles/moar/m.createLinePrefix(0xc00054c608, 0x4) github.com/walles/moar/m/screenLines.go:315 +0x315 github.com/walles/moar/m.(*Pager).decorateLine(0xc000616000, 0xc000500040?, {0xc000182e40, 0x2, 0x2}) github.com/walles/moar/m/screenLines.go:258 +0x90 github.com/walles/moar/m.(*Pager).renderLine(0xc000616000, 0x55bd9148cc17?, 0x3e8) github.com/walles/moar/m/screenLines.go:229 +0x295 github.com/walles/moar/m.(*scrollPositionInternal).emptyBottomLinesCount(0xc000616020, 0xc000616000) github.com/walles/moar/m/scrollPosition.go:168 +0x8e github.com/walles/moar/m.(*scrollPositionInternal).canonicalize(0xc000616020, 0xc000616000) github.com/walles/moar/m/scrollPosition.go:249 +0x189 github.com/walles/moar/m.(*Pager).lineNumberOneBased(...) github.com/walles/moar/m/scrollPosition.go:270 github.com/walles/moar/m.(*Pager).renderLines(0xc000616000) github.com/walles/moar/m/screenLines.go:137 +0x72 github.com/walles/moar/m.(*Pager).renderScreenLines(0xc000616000) github.com/walles/moar/m/screenLines.go:94 +0x2a github.com/walles/moar/m.(*Pager).redraw(0xc000616000, {0x0, 0x0}) github.com/walles/moar/m/screenLines.go:39 +0x53 github.com/walles/moar/m.(*Pager).StartPaging(0xc000616000, {0x55bd917b5d20?, 0xc000598000}) github.com/walles/moar/m/pager.go:539 +0x348 main.startPaging(0xc000582770?, {0x55bd917b5d20?, 0xc000598000?}) github.com/walles/moar/moar.go:549 +0x6f main.main() github.com/walles/moar/moar.go:509 +0x10d2 ```
walles commented 12 months ago

At least in theory it should be easy to just write a unit test for createLinePrefix() based on that lineNumberString <1000 > longer than numberPrefixLength 4 message and make it stop crashing.

walles commented 12 months ago

Turns out that function should never be called with 1000 and 4, it should be 1000 and 5 so the problem is somewhere else.

Repro:

  1. Make your terminal window 60 high
  2. Open services.txt.gz in moar (it's from /etc/services on my laptop)
  3. Search (/) for netware. The crash comes after netwar when line 1000 is just outside of the screen.
postsolar commented 12 months ago

Couldn't reproduce it from your instructions (perhaps due to words with netwar* being on different lines for me?), but if you imply it's something about the line 1000, then yes, I just reproduced it slightly differently:

  1. Terminal size 21 x 90
  2. man tmux
  3. Search for select-, accept the search with return
  4. Hit n 3 times
  5. Start scrolling up
  6. The moment line 1000 goes below the lower border of the terminal it crashes
Details Please post the following report at , or e-mail it to johan.walles@gmail.com. Version: 1.18.4 LANG : en_US.UTF-8 TERM : foot-extra GOOS : linux GOARCH : amd64 Compiler: gc NumCPU : 4 panic: lineNumberString <1000 > longer than numberPrefixLength 4 [recovered] panic: lineNumberString <1000 > longer than numberPrefixLength 4 [recovered] panic: lineNumberString <1000 > longer than numberPrefixLength 4 goroutine 1 [running]: main.main.func1() github.com/walles/moar/moar.go:333 +0x155 panic({0x55a6294c98e0?, 0xc000416670?}) runtime/panic.go:914 +0x21f main.startPaging.func1() github.com/walles/moar/moar.go:538 +0xb0 panic({0x55a6294c98e0?, 0xc000416670?}) runtime/panic.go:914 +0x21f github.com/walles/moar/m.createLinePrefix(0xc000013520, 0x4) github.com/walles/moar/m/screenLines.go:315 +0x315 github.com/walles/moar/m.(*Pager).decorateLine(0xc000130000, 0xc0000d01b0?, {0xc0001aa840, 0x2, 0x2}) github.com/walles/moar/m/screenLines.go:258 +0x90 github.com/walles/moar/m.(*Pager).renderLine(0xc000130000, 0x55a6291d0c17?, 0x3e8) github.com/walles/moar/m/screenLines.go:229 +0x295 github.com/walles/moar/m.(*scrollPositionInternal).emptyBottomLinesCount(0xc000130020, 0xc000130000) github.com/walles/moar/m/scrollPosition.go:168 +0x8e github.com/walles/moar/m.(*scrollPositionInternal).canonicalize(0xc000130020, 0xc000130000) github.com/walles/moar/m/scrollPosition.go:249 +0x189 github.com/walles/moar/m.(*Pager).lineNumberOneBased(...) github.com/walles/moar/m/scrollPosition.go:270 github.com/walles/moar/m.(*Pager).renderLines(0xc000130000) github.com/walles/moar/m/screenLines.go:137 +0x72 github.com/walles/moar/m.(*Pager).renderScreenLines(0xc000130000) github.com/walles/moar/m/screenLines.go:94 +0x2a github.com/walles/moar/m.(*Pager).redraw(0xc000130000, {0x0, 0x0}) github.com/walles/moar/m/screenLines.go:39 +0x53 github.com/walles/moar/m.(*Pager).StartPaging(0xc000130000, {0x55a6294f9d20?, 0xc000097380}) github.com/walles/moar/m/pager.go:539 +0x348 main.startPaging(0xc0002509a0?, {0x55a6294f9d20?, 0xc000097380?}) github.com/walles/moar/moar.go:549 +0x6f main.main() github.com/walles/moar/moar.go:509 +0x10d2
walles commented 12 months ago

I've collected a number of crashes now, and the common part of the stack trace is as below.

Basically somebody calls lineNumberOneBased() with some input and it crashes.

So that's where the new test is needed.

main.main.func1()
    github.com/walles/moar/moar.go:333 +0x155
panic({0x11e4ca0?, 0xc0004081e0?})
    runtime/panic.go:914 +0x21f
main.startPaging.func1()
    github.com/walles/moar/moar.go:538 +0xb0
panic({0x11e4ca0?, 0xc0004081e0?})
    runtime/panic.go:914 +0x21f
github.com/walles/moar/m.createLinePrefix(0xc0001ea0c0, 0x4)
    github.com/walles/moar/m/screenLines.go:315 +0x315
github.com/walles/moar/m.(*Pager).decorateLine(0xc0000ae000, 0xc00048a0c0?, {0xc000477900, 0x92, 0x92})
    github.com/walles/moar/m/screenLines.go:258 +0x90
github.com/walles/moar/m.(*Pager).renderLine(0xc0000ae000, 0xc0004ef5c0?, 0x3e8)
    github.com/walles/moar/m/screenLines.go:229 +0x295
github.com/walles/moar/m.(*scrollPositionInternal).emptyBottomLinesCount(0xc0004ef708, 0xc0000ae000)
    github.com/walles/moar/m/scrollPosition.go:168 +0x8e
github.com/walles/moar/m.(*scrollPositionInternal).canonicalize(0xc0004ef708, 0xc0000ae000)
    github.com/walles/moar/m/scrollPosition.go:249 +0x189
github.com/walles/moar/m.(*scrollPosition).lineNumberOneBased(...)
    github.com/walles/moar/m/scrollPosition.go:276
walles commented 12 months ago

Should be solved in just-out 1.18.5: https://github.com/walles/moar/releases/tag/v1.18.5

HomeBrew build status is here: https://github.com/Homebrew/homebrew-core/pull/156194

Thanks for your reports @postsolar & @andyrtr!