Misbehavior and crash when output is cleared and redrawn while some text is selected #633

Closed
opened 2 months ago by Arnavion · 2 comments
foot version: 1.8.1 -pgo +ime +graphemes

Repro:

while sleep 1; do clear; for i in {0..100}; do echo "$i"; done; done

(Adjust 100 to be larger than the number of columns visibile in one page of the window. That is, when the script runs, there must be some lines in the scrollback.)

Run this script. It'll print some lines ending with 100. Select one of the numbers with a double-click, say 99, and wait.

Problem 1: When the loop runs again -> clears the screen -> rewrites the numbers, the selection moves up a few lines instead of clearing. Eg for me, the selection moves from 99 to 70, then to 41, then off-screen. Instead the selection should probably clear itself.

Problem 2: Eventually, foot crashes. Probably when the phantom selection moves before the off-screen 1.


gdb says (optimized build; I haven't tried with a debug build yet):

Thread 1 "foot" received signal SIGSEGV, Segmentation fault.
foreach_selected_normal (data=0x7fffffffd380, cb=0x55555557bd90 <mark_selected>, _end=...,
_start=..., term=0x555555679480) at ../selection.c:145

(gdb) bt

#0  foreach_selected_normal (data=0x7fffffffd380, cb=0x55555557bd90 <mark_selected>, _end=..., _start=..., term=0x555555679480) at ../selection.c:145
#1  foreach_selected (term=term@entry=0x555555679480, start=..., end=..., cb=cb@entry=0x55555557bd90 <mark_selected>, data=data@entry=0x7fffffffd380) at ../selection.c:194
#2  0x0000555555581cef in selection_dirty_cells (term=0x555555679480) at ../selection.c:779
#3  selection_dirty_cells (term=0x555555679480) at ../selection.c:774
#4  grid_render.part.0.lto_priv.0 (term=0x555555679480) at ../render.c:2383
#5  0x000055555557b4f2 in grid_render (term=0x555555679480) at ../render.c:2280
#6  fdm_hook_refresh_pending_terminals (fdm=<optimized out>, data=<optimized out>) at ../render.c:3542
#7  0x0000555555569c8b in fdm_poll (fdm=0x5555556616c0) at ../fdm.c:415
#8  main (argc=<optimized out>, argv=<optimized out>) at ../main.c:554

(gdb) info locals

c = 0
real_r = <optimized out>
row = 0x0
r = 166
end = <synthetic pointer>
start_row = <optimized out>
start_col = <optimized out>
start = <synthetic pointer>
end_row = <optimized out>
end_col = <optimized out>
start = <optimized out>
end = <optimized out>
start_row = <optimized out>
end_row = <optimized out>
start_col = <optimized out>
end_col = <optimized out>
r = <optimized out>
real_r = <optimized out>
row = <optimized out>
c = <optimized out>

(gdb) p cb

$1 = (_Bool (*)(struct terminal *, struct row *, struct cell *, int, void *)) 0x55555557bd90 <mark_selected>
``` foot version: 1.8.1 -pgo +ime +graphemes ``` --- Repro: ```sh while sleep 1; do clear; for i in {0..100}; do echo "$i"; done; done ``` (Adjust 100 to be larger than the number of columns visibile in one page of the window. That is, when the script runs, there must be some lines in the scrollback.) Run this script. It'll print some lines ending with `100`. Select one of the numbers with a double-click, say `99`, and wait. Problem 1: When the loop runs again -> clears the screen -> rewrites the numbers, the selection moves up a few lines instead of clearing. Eg for me, the selection moves from `99` to `70`, then to `41`, then off-screen. Instead the selection should probably clear itself. Problem 2: Eventually, foot crashes. Probably when the phantom selection moves before the off-screen `1`. --- gdb says (optimized build; I haven't tried with a debug build yet): ``` Thread 1 "foot" received signal SIGSEGV, Segmentation fault. foreach_selected_normal (data=0x7fffffffd380, cb=0x55555557bd90 <mark_selected>, _end=..., _start=..., term=0x555555679480) at ../selection.c:145 (gdb) bt #0 foreach_selected_normal (data=0x7fffffffd380, cb=0x55555557bd90 <mark_selected>, _end=..., _start=..., term=0x555555679480) at ../selection.c:145 #1 foreach_selected (term=term@entry=0x555555679480, start=..., end=..., cb=cb@entry=0x55555557bd90 <mark_selected>, data=data@entry=0x7fffffffd380) at ../selection.c:194 #2 0x0000555555581cef in selection_dirty_cells (term=0x555555679480) at ../selection.c:779 #3 selection_dirty_cells (term=0x555555679480) at ../selection.c:774 #4 grid_render.part.0.lto_priv.0 (term=0x555555679480) at ../render.c:2383 #5 0x000055555557b4f2 in grid_render (term=0x555555679480) at ../render.c:2280 #6 fdm_hook_refresh_pending_terminals (fdm=<optimized out>, data=<optimized out>) at ../render.c:3542 #7 0x0000555555569c8b in fdm_poll (fdm=0x5555556616c0) at ../fdm.c:415 #8 main (argc=<optimized out>, argv=<optimized out>) at ../main.c:554 (gdb) info locals c = 0 real_r = <optimized out> row = 0x0 r = 166 end = <synthetic pointer> start_row = <optimized out> start_col = <optimized out> start = <synthetic pointer> end_row = <optimized out> end_col = <optimized out> start = <optimized out> end = <optimized out> start_row = <optimized out> end_row = <optimized out> start_col = <optimized out> end_col = <optimized out> r = <optimized out> real_r = <optimized out> row = <optimized out> c = <optimized out> (gdb) p cb $1 = (_Bool (*)(struct terminal *, struct row *, struct cell *, int, void *)) 0x55555557bd90 <mark_selected> ```
Owner

Nice report, thanks!

clear comes down to what's basically three escape sequences: it "homes" the cursor, erases the screen and finally erases the scrollback.

The last thing is what causes the crash. If you run clear while the selection is in the scrollback, rather than on screen, we crash.

This is an abvious bug - while rendering, we try to de-reference the rows the current selection points to. When the scrollback is erased, those rows are not just erased, but deleted. Thus we get a NULL pointer de-reference.

As for the selection scrolling up... that's kind of expected. The selection isn't cleared when the cells it touches are erased, or printed to. So, after the clear, the selection is still where you first made it (e.g. at 99). Then you print the 100 lines. This causes the screen content to scroll up by (100 - "visible rows"). And the selection always scrolls with the content (anything else would be weird, I think).

Now, one can argue that the selection should be removed if the selected text is erased, or overwritten. However, I don't think we want that. Consider an application that updates the screen periodically, htop for example. If the selection was cancelled as soon as something in it was erased or replaced, it'd be almost impossible to select anything at all.

Nice report, thanks! `clear` comes down to what's basically three escape sequences: it "homes" the cursor, erases the screen and finally erases the scrollback. The last thing is what causes the crash. If you run clear while the selection is in the scrollback, rather than on screen, we crash. This is an abvious bug - while rendering, we try to de-reference the rows the current selection points to. When the scrollback is erased, those rows are not just erased, but deleted. Thus we get a NULL pointer de-reference. As for the selection scrolling up... that's kind of expected. The selection isn't cleared when the cells it touches are erased, or printed to. So, after the `clear`, the selection is still where you first made it (e.g. at `99`). Then you print the 100 lines. This causes the screen content to scroll up by (100 - _"visible rows"_). And the selection always scrolls with the content (anything else would be weird, I think). Now, one can argue that the selection should be removed if the selected text is erased, or overwritten. However, I don't think we want that. Consider an application that updates the screen periodically, `htop` for example. If the selection was cancelled as soon as something in it was erased or replaced, it'd be almost impossible to select anything at all.
dnkl added the
bug
label 2 months ago
Poster

Your explanation of the behavior of the selection makes sense and I agree that it should stay.

Your explanation of the behavior of the selection makes sense and I agree that it should stay.
dnkl closed this issue 2 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.