Crash while yanking text

[Andersama]

Summary

Helix crashed as I yanked text on a left vertical split. Not sure that I can replicate it, it was after quite a while of editing. Was using the diagnostics menu several times over, <ctrl+o> to jump around. Getting used to editing with helix. Not familiar with rust so while I'd love to debug this myself, the error reads like a vector out of bounds assert failing in c++. The two files in vertical panes were different sizes I'd almost suspect something checked a selection offset against the size of the wrong file. I had been copy / pasting between the two panes but I hadn't yet learned the commands to switch between the panes so I was switching between the two using the mouse.

thread 'main' panicked at 'called Result::unwrap() on an Err value: Char index out of bounds: char index 175088, Rope/RopeSlice char length 47656', C:\Users\runneradmin.cargo\registry\src\github.com-1ecc6299db9ec823\ropey-1.5.0\src\slice.rs:349:41

Reproduction Steps

No response

Helix log

helix.log "project_name" is what I was working on last 2022-10-08T00:39:14.051 was the last timestamp

Platform

Windows

Terminal Emulator

cmd

Helix Version

helix 22.08.1 (66276ce6)

[Ordoviz]

Possibly related: #4157

[Andersama]

Fairly confident my initial guess was correct, I just took a quick look at the file sizes of the two files I was editing at the time. 47656 is roughly the size of the target file I was yanking from and the char index is around in the other file where I was editing.

I think I found the area that might be responsible for the bug, this appears to be the only reference to yank being called and it's to do with handling mouse events. There's likely a problem where the "current" view is not updated in time with the mouse click. It could be there's a missing mouse down handler or mouse up handler to make sure the last clicked pane is the "current" one used for the remaining function calls.

https://github.com/helix-editor/helix/blob/c15f1ea274d300feb23208324aa5b27d7274bebd/helix-term/src/ui/editor.rs#L1197

            MouseEventKind::Up(MouseButton::Left) => {
                if !config.middle_click_paste {
                    return EventResult::Ignored(None);
                }

                let (view, doc) = current!(cxt.editor);
               
                if doc
                    .selection(view.id)
                    .primary()
                    .slice(doc.text().slice(..))
                    .len_chars()
                    <= 1
                {
                    return EventResult::Ignored(None);
                }

                commands::MappableCommand::yank_main_selection_to_primary_clipboard.execute(cxt);

                EventResult::Consumed(None)
            }

[kirawi]

It might need a view.ensure_cursor_in_view(doc, scrolloff);

[Andersama]

I'm not entirely sure this would be something quick to fix, I might be wrong.

In theory, whatever happened to me should never happen*, because what it suggests is that the yank command (and I'm assuming other commands will likely have this problem as well) can try to run on what is effectively stale data. If I "yank" or do any command the command itself should be paired not only with an offset and size but with the panel it was executed on, so that when it executes later (as it seems selections don't carry this information and is instead merged with the "current" view) it can be sure to do exactly what the user intended at the time it was typed (or clicked). This would suggest select operations would likely need a smart pointer to refer to the panel and there needs to be some check that the "current" panel matches before any operation.

EG: I'd imagine there's already a bug which allows me to try to yank, some other thing closes a panel and then have the yank command run on the wrong panel and continue on like nothing happened. Provided every command can be properly undone and/or you've got version control maybe that's fine. But in the couple of days I've been playing around with vim and helix I've managed to do some incredibly destructive edits by accident so I can only imagine this bug causing someone a real headache in the future. Crashing also the way it did could also be a major pain point, if I wasn't already in the habit of saving my work as I go it could easily be that a crash like this undoes hours of someone's work.

[Andersama]

I think*, learning a bit of rust here something like:

    let selected = doc.selection(view.id);
    let ranges = selected.ranges();
    let out_of_bounds = ranges.iter().any(|&r| r.to() > text.len_bytes());

Might detect the problem before things go horribly wrong.

[David-Else]

@Andersama Thanks for looking into this, have you considered a pull request? I am sure someone will be able to help you with the finer details of Rust.

[Andersama]

I'm somewhat hoping / crossing fingers that someone can confirm the bug is fixed by something else. What I saw when looking at the code was a lot of areas which might need a check like this. I'd be worried writing a pr that I'd miss something. My code's also doing a brute force range check, that could have a performance impact.

I'm already getting my around rust, not a fan, but it's ok.

[Andersama]

I think this might be the cause of a few other issues* #3978 looks like a similar crash.

[Jummit]

This is still a problem, here is my stack trace:

thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Char index out of bounds: char index 2220, Rope/RopeSlice char length 589', /home/jummit/.local/share/cargo/registry/src/github.com-1ecc6299db9ec823/ropey-1.5.0/src/slice.rs:349:41
stack backtrace:
   0: rust_begin_unwind
             at /rustc/fe5b13d681f25ee6474be29d748c65adcd91f69e/library/std/src/panicking.rs:584:5
   1: core::panicking::panic_fmt
             at /rustc/fe5b13d681f25ee6474be29d748c65adcd91f69e/library/core/src/panicking.rs:143:14
   2: core::result::unwrap_failed
             at /rustc/fe5b13d681f25ee6474be29d748c65adcd91f69e/library/core/src/result.rs:1785:5
   3: ropey::slice::RopeSlice::char_to_byte
   4: helix_core::graphemes::nth_prev_grapheme_boundary
   5: helix_core::selection::Range::grapheme_aligned
   6: helix_core::selection::Selection::ensure_invariants
   7: helix_view::document::Document::set_selection
   8: helix_term::commands::jump_backward
   9: helix_term::ui::editor::EditorView::handle_keymap_event::{{closure}}
  10: helix_term::ui::editor::EditorView::handle_keymap_event
  11: <helix_term::ui::editor::EditorView as helix_term::compositor::Component>::handle_event
  12: helix_term::compositor::Compositor::handle_event
  13: helix_term::application::Application::handle_terminal_events
  14: helix_term::application::Application::event_loop_until_idle::{{closure}}
  15: <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll
  16: std::thread::local::LocalKey<T>::with
  17: tokio::park::thread::CachedParkThread::block_on
  18: tokio::runtime::scheduler::multi_thread::MultiThread::block_on
  19: tokio::runtime::Runtime::block_on
  20: hx::main

[creikey]

Have this same bug with a much smaller file: thread 'main' panicked at 'Attempt to slice past end of RopeSlice: slice end 465, RopeSlice length 463', C:\Users\Cameron\.cargo\registry\src\github.com-1ecc6299db9ec823\ropey-1.5.0\src\slice.rs:645:9

[pascalkuthe]

I am fairly certain that #5790 fixes this so I am going to close this issue. If you are still having problems feel free to open a new issue.

[Andersama]

@pascalkuthe It sounds like from your patch that the issue was a form of desynchronization of the edit history. Without digging too deep into the source code, does the history of edits include which panel / screen the edit was made on? Because the reason I suspected there was some underlying bug was in part because keyboard actions should be somewhat processed instantly, therefore you really shouldn't have out of bounds errors given the gui's current state because it's already there. I didn't think about edit history which could make things interesting.

What happens say if you perform some actions -> they get logged into the edit history, then you remove a panel where an edit was made and you try to undo?