:reload-all crashes with thread 'main' panicked at 'Position 5541 is out of range for changeset len 4994!', helix-core/src/transaction.rs:397:13

[ocharles]

Summary

While editing, I ran :reload-all and Helix crashed with:

thread 'main' panicked at 'Position 5541 is out of range for changeset len 4994!', helix-core/src/transaction.rs:397:13
stack backtrace:
   0: rust_begin_unwind
   1: core::panicking::panic_fmt
   2: helix_core::transaction::ChangeSet::map_pos
   3: <smallvec::SmallVec<A> as core::iter::traits::collect::Extend<<A as smallvec::Array>::Item>>::extend
   4: helix_core::selection::Selection::map
   5: helix_view::view::View::apply
   6: helix_view::document::Document::append_changes_to_history
   7: helix_view::document::Document::reload
   8: helix_term::commands::typed::reload_all
   9: helix_term::commands::typed::command_mode::{{closure}}
  10: <helix_term::ui::prompt::Prompt as helix_term::compositor::Component>::handle_event
  11: helix_term::compositor::Compositor::handle_event
  12: helix_term::application::Application::run::{{closure}}
  13: <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll
  14: tokio::runtime::park::CachedParkThread::block_on
  15: tokio::runtime::scheduler::multi_thread::MultiThread::block_on
  16: tokio::runtime::runtime::Runtime::block_on
  17: hx::main
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

Reproduction Steps

I don't have exact steps to reproduce. I had a few files open, and a vertical split. The file on the left had focus, and was changed outside of Helix, so I was expecting that file to update after :reload-all.

Helix log

No response

Platform

Linux

Terminal Emulator

Kitty

Helix Version

5a3ff74

[diktomat]

Just had the same panic (macOS, also Kitty, 22.08.1-501-ge6dad960), but in the middle of inputting some Rust, without running any commands or doing something special.

[archseer]

@pascalkuthe

[archseer]

Ah wait, this could also be a symptom of :reload-all

[pascalkuthe]

@archseer just from the backtrace without a reproduction step this difficult to disect. It's possible that I missed something in the reload implementation. I will take a look again. I have been experiencing similar crashes but these should be fixed by #4912. I will switch to the latest master and stresstest a bit in the hopes of reproducing this

[pascalkuthe]

I apologise for my confused previous comments, the char_idx <-> byte_idx conversion errror was in my fuzzing setup and not in the code itself.
I have been running the fuzzer for quite a while now and I have gotten no crashes. I will it run for another hour just to be sure but these kinds of bugs usually turn up much quicker. I am fairly certain now that helix_core::diff::compare_ropes always produces a correct transaction and is not causing the bug.

However as I mentioned I initially taught I had reproduced the bug because I got the exact same panic message as shown above. That happend because I was using a bytes instead of a char bounds check in the fuzzer which lead to producing a position that was past the end of the original text.

I suspect something similar is happening here so the problem is actually the selection that is being transformed with map_pos and not the Transaction/ChangeSet. Maybe a similar problem to #4888?

However I have been unable to narrow this down further without any reproduction steps. I tried reproducing by doing a bunch of reloads but nothing happens.

[the-mikedavis]

Ah, I reproduced this. I don't think it's related to the diffing change.

1. hx foo.txt
2. [<space>[<space>[<space> create three lines so that the cursor is at the end
3. :w<ret> so this exists on disk
4. <C-w>v open a split for this file
5. <C-s> to save a spot in the jumplist on window 2
6. <C-w>w switch back to window 1
7. %d delete the whole buffer
8. :reload-all<ret> (panics)

The issue is that reload_all calls Document::reload for the (doc, view) pairs for each window. The second window is behind on the document's history (the %d in this case) and we don't catch it up before applying the new reload transaction. We should be able to fix this by adding a view.sync_changes(doc) before the call to Document::reload.