Crash when terminal is reset while in scrollback search mode #644

Closed
opened 3 months ago by kahatlen · 4 comments

I occasionally see crashes when hitting Ctrl-w while in scrollback search mode. I don't know exactly what has triggered the crashes I've seen in the wild. Maybe it was Emacs or tmux that did a repainting of the screen while scrollback search was active, or maybe something else. In any case, I have been able to construct an artificial case that consistently crashes for me. These are the steps:

  1. In a foot terminal, execute echo abc; sleep 10; reset.
  2. During the 10-second sleep period, enter scrollback search with Shift-Ctrl-r, and type "abc" (without the quotes, and not hitting return after entering the string)
  3. After the reset command has run, hit Ctrl-w. Foot crashes.

Backtrace:

(gdb) bt
#0  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
#1  0x00007ffff79ab537 in __GI_abort () at abort.c:79
#2  0x00005555555c4b3f in bug (file=0x5555555d767d "search.c", line=440, func=0x5555555d7a70 <__func__.3> "search_match_to_end_of_word", fmt=0x5555555d7666 "assertion failed: '%s'") at ../debug.c:46
#3  0x000055555558f7eb in search_match_to_end_of_word (term=0x5555556124a0, spaces_only=false) at ../search.c:440
#4  0x00005555555907c8 in execute_binding (seat=0x55555561cfe0, term=0x5555556124a0, action=BIND_ACTION_SEARCH_EXTEND_WORD, serial=13908, update_search_result=0x7fffffffcf97, redraw=0x7fffffffcf96) at ../search.c:760
#5  0x00005555555909ea in search_input (seat=0x55555561cfe0, term=0x5555556124a0, key=25, sym=119, mods=4, consumed=1, raw_syms=0x5555556a0f48, raw_count=1, serial=13908) at ../search.c:815
#6  0x000055555557c28d in key_press_release (seat=0x55555561cfe0, term=0x5555556124a0, serial=13908, key=25, state=1) at ../input.c:1000
#7  0x000055555557cace in keyboard_key (data=0x55555561cfe0, wl_keyboard=0x555555610e70, serial=13908, time=26006242, key=17, state=1) at ../input.c:1221
#8  0x00007ffff7980d1d in ?? () from /lib/x86_64-linux-gnu/libffi.so.7
#9  0x00007ffff7980289 in ?? () from /lib/x86_64-linux-gnu/libffi.so.7
#10 0x00007ffff7db52d2 in ?? () from /lib/x86_64-linux-gnu/libwayland-client.so.0
#11 0x00007ffff7db197a in ?? () from /lib/x86_64-linux-gnu/libwayland-client.so.0
#12 0x00007ffff7db303c in wl_display_dispatch_queue_pending () from /lib/x86_64-linux-gnu/libwayland-client.so.0
#13 0x000055555559d0e5 in fdm_wayl (fdm=0x55555561a6e0, fd=5, events=1, data=0x555555608700) at ../wayland.c:1121
#14 0x0000555555577e35 in fdm_poll (fdm=0x55555561a6e0) at ../fdm.c:463
#15 0x00005555555808eb in main (argc=0, argv=0x7fffffffe550) at ../main.c:554

It hits this assertion:

(gdb) f 3
#3  0x000055555558f7eb in search_match_to_end_of_word (term=0x5555556124a0, spaces_only=false) at ../search.c:440
440	   xassert(term->selection.end.row != -1);

Tested with:

 % ./foot --version
foot version: 1.8.2-7-ga987b1b (Jul 22 2021, branch 'master') -pgo +ime -graphemes
I occasionally see crashes when hitting Ctrl-w while in scrollback search mode. I don't know exactly what has triggered the crashes I've seen in the wild. Maybe it was Emacs or tmux that did a repainting of the screen while scrollback search was active, or maybe something else. In any case, I have been able to construct an artificial case that consistently crashes for me. These are the steps: 1. In a foot terminal, execute `echo abc; sleep 10; reset`. 2. During the 10-second sleep period, enter scrollback search with Shift-Ctrl-r, and type "abc" (without the quotes, and not hitting return after entering the string) 3. After the `reset` command has run, hit Ctrl-w. Foot crashes. Backtrace: ``` (gdb) bt #0 __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50 #1 0x00007ffff79ab537 in __GI_abort () at abort.c:79 #2 0x00005555555c4b3f in bug (file=0x5555555d767d "search.c", line=440, func=0x5555555d7a70 <__func__.3> "search_match_to_end_of_word", fmt=0x5555555d7666 "assertion failed: '%s'") at ../debug.c:46 #3 0x000055555558f7eb in search_match_to_end_of_word (term=0x5555556124a0, spaces_only=false) at ../search.c:440 #4 0x00005555555907c8 in execute_binding (seat=0x55555561cfe0, term=0x5555556124a0, action=BIND_ACTION_SEARCH_EXTEND_WORD, serial=13908, update_search_result=0x7fffffffcf97, redraw=0x7fffffffcf96) at ../search.c:760 #5 0x00005555555909ea in search_input (seat=0x55555561cfe0, term=0x5555556124a0, key=25, sym=119, mods=4, consumed=1, raw_syms=0x5555556a0f48, raw_count=1, serial=13908) at ../search.c:815 #6 0x000055555557c28d in key_press_release (seat=0x55555561cfe0, term=0x5555556124a0, serial=13908, key=25, state=1) at ../input.c:1000 #7 0x000055555557cace in keyboard_key (data=0x55555561cfe0, wl_keyboard=0x555555610e70, serial=13908, time=26006242, key=17, state=1) at ../input.c:1221 #8 0x00007ffff7980d1d in ?? () from /lib/x86_64-linux-gnu/libffi.so.7 #9 0x00007ffff7980289 in ?? () from /lib/x86_64-linux-gnu/libffi.so.7 #10 0x00007ffff7db52d2 in ?? () from /lib/x86_64-linux-gnu/libwayland-client.so.0 #11 0x00007ffff7db197a in ?? () from /lib/x86_64-linux-gnu/libwayland-client.so.0 #12 0x00007ffff7db303c in wl_display_dispatch_queue_pending () from /lib/x86_64-linux-gnu/libwayland-client.so.0 #13 0x000055555559d0e5 in fdm_wayl (fdm=0x55555561a6e0, fd=5, events=1, data=0x555555608700) at ../wayland.c:1121 #14 0x0000555555577e35 in fdm_poll (fdm=0x55555561a6e0) at ../fdm.c:463 #15 0x00005555555808eb in main (argc=0, argv=0x7fffffffe550) at ../main.c:554 ``` It hits this assertion: ``` (gdb) f 3 #3 0x000055555558f7eb in search_match_to_end_of_word (term=0x5555556124a0, spaces_only=false) at ../search.c:440 440 xassert(term->selection.end.row != -1); ``` Tested with: ``` % ./foot --version foot version: 1.8.2-7-ga987b1b (Jul 22 2021, branch 'master') -pgo +ime -graphemes ```
Owner

Thanks for taking the time to find a way to reproduce, and writing up such a detailed report! Appreciate it!

Now, I think it's pretty clear the problem is that the selection has been cancelled (typically due to the selection being scrolled out), and that ctrl+w doesn't handle this.

That in itself should be easy to fix, and I might even push a fix for it.

However, I'm wondering if it's not a better long term solution to "freeze" the output while scrollback search is active, much like we do in URL mode. I.e. changes happening while scrollback search is active isn't reflected on screen, and the scrollback search itself works on the snapshotted content.

This would also make it easier to capture quickly changing content. If I only do the fix mentioned above, your current scrollback search selection would keep "disappearing" (due to the selection being cancelled).

A possible downside is you'd think the content is un-changing, but the moment you press Return, you exit search mode and all the changes that have been held back are applied at once, and your selection is either immediately cancelled, or changed to something else.

This means you'd have to rely on the search selection being copied to the primary selection, since exiting scrollback search and then copying it (with ctrl+shift+c) wouldn't work, or would copy something else. On the other hand, it kind of already is like that.

Not sure how easy it will be to snapshot the grid in scrollback search mode, but I think it's worth investigating.

Thanks for taking the time to find a way to reproduce, and writing up such a detailed report! Appreciate it! Now, I think it's pretty clear the problem is that the selection has been cancelled (typically due to the selection being scrolled out), and that <kbd>ctrl</kbd>+<kbd>w</kbd> doesn't handle this. That in itself should be easy to fix, and I might even push a fix for it. However, I'm wondering if it's not a better long term solution to "freeze" the output while scrollback search is active, much like we do in URL mode. I.e. changes happening while scrollback search is active isn't reflected on screen, and the scrollback search itself works on the snapshotted content. This would also make it easier to capture quickly changing content. If I _only_ do the fix mentioned above, your current scrollback search selection would keep "disappearing" (due to the selection being cancelled). A possible downside is you'd _think_ the content is un-changing, but the moment you press <kbd>Return</kbd>, you exit search mode and all the changes that have been held back are applied at once, and your selection is either immediately cancelled, or changed to something else. This means you'd have to rely on the search selection being copied to the primary selection, since exiting scrollback search and _then_ copying it (with <kbd>ctrl</kbd>+<kbd>shift</kbd>+<kbd>c</kbd>) wouldn't work, or would copy something else. On the other hand, it kind of already _is_ like that. Not sure how easy it will be to snapshot the grid in scrollback search mode, but I think it's worth investigating.
dnkl added the
bug
label 3 months ago
Owner

#645 should fix the crash. It does not implement the snapshot logic described above, but instead simply resets the scrollback search "match" state when the selection is cancelled.

It fixes your test case, but feel free to give it a spin and see if you can make it crash ;)

https://codeberg.org/dnkl/foot/pulls/645 should fix the crash. It does not implement the snapshot logic described above, but instead simply resets the scrollback search "match" state when the selection is cancelled. It fixes your test case, but feel free to give it a spin and see if you can make it crash ;)
Owner

Not sure how easy it will be to snapshot the grid in scrollback search mode, but I think it's worth investigating.

This is now #646

> Not sure how easy it will be to snapshot the grid in scrollback search mode, but I think it's worth investigating. This is now https://codeberg.org/dnkl/foot/issues/646
Poster

Thanks for the quick response! #645 does indeed seem to fix the immediate problem. I agree that freezing the output would make the behaviour nicer. Searching against a moving target can cause surprises, so snapshotting the scrollback, or somehow pausing the processing of the output from the application while in scrollback search mode, sounds like a good idea from a usability point of view.

Thanks for the quick response! #645 does indeed seem to fix the immediate problem. I agree that freezing the output would make the behaviour nicer. Searching against a moving target can cause surprises, so snapshotting the scrollback, or somehow pausing the processing of the output from the application while in scrollback search mode, sounds like a good idea from a usability point of view.
dnkl closed this issue 3 months ago
Sign in to join this conversation.
No Milestone
No Assignees
2 Participants
Notifications
Due Date

No due date set.

Dependencies

This issue currently doesn't have any dependencies.

Loading…
There is no content yet.