helix icon indicating copy to clipboard operation
helix copied to clipboard

'Position ... is out of range for changeset len ....!' panic on ~23.03 when saving a file

Open bcspragu opened this issue 2 years ago • 8 comments

Summary

Helix panicked on a recent build while saving a file, with error:

thread 'main' panicked at 'Position 6234 is out of range for changeset len 5108!', helix-core/src/transaction.rs:399:13

I was saving some combination of Go + Vue 2 files at the time

Reproduction Steps

Unfortunately I don't have a reproducible use case, this is the first panic I've had since updating to something around 23.03

Helix log

The helix.log didn't have anything useful, just LSP-related errors that aren't actually problems.

Platform

Linux

Terminal Emulator

alacritty / tmux

Helix Version

helix 23.03 (3cf03723)

bcspragu avatar Apr 14 '23 02:04 bcspragu

I am working on the steps to reproduce, for now here is a backtrace of a similar case when changing views.

thread 'main' panicked at 'Position 2 is out of range for changeset len 1!', helix-core/src/transaction.rs:399:13
stack backtrace:
   0: rust_begin_unwind
             at /rustc/897e37553bba8b42751c67658967889d11ecd120/library/std/src/panicking.rs:584:5
   1: core::panicking::panic_fmt
             at /rustc/897e37553bba8b42751c67658967889d11ecd120/library/core/src/panicking.rs:142:14
   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::view::View::sync_changes
   7: helix_view::editor::Editor::replace_document_in_view
   8: helix_view::editor::Editor::switch
   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: <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

trink avatar Apr 14 '23 14:04 trink

@trink what you are running into seems to be #5632. I am not sure if that's the same crash as this issue (and can currently not be fixed). Thanks for looking into this regardless!

The issue as originally reported could be anything as this is the basic "something went wrong" crash but without a backtrace or reproduction case we have no idea what. @bcspragu are you saving in insert mode with a keybinding? That tends to cause crashes like this

pascalkuthe avatar Apr 14 '23 14:04 pascalkuthe

In this case I just switched buffers using 'ga', there were no splits or prompts open. Isolating where/when things got out of sync is proving challenging but I am not clear on why it cannot be fixed... should I just move on to investigating my next panic?

trink avatar Apr 14 '23 15:04 trink

In this case I just switched buffers using 'ga', there were no splits or prompts open. Isolating where/when things got out of sync is proving challenging but I am not clear on why it cannot be fixed... should I just move on to investigating my next panic?

Hmm maybe I misinterpreted the backtrace. https://github.com/helix-editor/helix/issues/5632 is about various popups (like the regex selection) storing positions in the document. When you switch buffers while the popup is open (with the mouse for example) you end up with a different document open and therefore the position stored in the popup is outdated and can cause crashes like this. That bug cannot be fixed because it requires access to the compositor when switching views in helix-view. Changing the architecture of helix like that is a pretty big task. We want to get to it eventually but we haven't settled on a solution yet.

Teh traceback you reported above might be a different bug though, looking at the backtrace again. I think I confused some method names earlier and assumed that you had a regex popup open. I am still not sure if it's the same crash as the original issue since that occurred while saving. Like I said the crash could be anything but that ultimately doesn't matter if you found a different panic. Tracking down and fixing panics is always appreciated!

So if you can manage a reproduction case for this that would be great @trink sorry about the confusionn.

pascalkuthe avatar Apr 14 '23 15:04 pascalkuthe

@bcspragu are you saving in insert mode with a keybinding? That tends to cause crashes like this

I believe I was just doing a normal :wa when it crashed

bcspragu avatar Apr 14 '23 16:04 bcspragu

@bcspragu are you saving in insert mode with a keybinding? That tends to cause crashes like this

I believe I was just doing a normal :wa when it crashed

The fact that you were using :wa helps that command is new. I think I have a pretty good guess as to what's causing this. I am assuming that you have autoformatting (format on save) enabled? It's enabled by default unless you set the option explicitly to false. When saving the autoformat code is called. The autoformat code was originally implemented for :w (before :wa was added) under the assumption that it always applies to the currently open view.

However :wa also saves hidden buffers/buffers open in another view. The view state is only ever up to date for the focused view which is not considered in that code. For example we call ensure_cursor_in_view when applying the auto-formatting. This wil mess up the view position and at best this just causes the view to jump weirdly at worst it causes crashes (although this is not the crash you ran into since that has nothing to do with transactions).

The cause for the crash you ran into happens even earlier (just before the function that would cause the other bug is called). We call append_changes_to_history in the format callback but append_changes_to_history assumes that the view is synced to the newest document revision but this is not necessarily the case for :wa. To fix this we could make append_changes_to_history call View::sync_changes but that would be a bit inefficient considering how often that function is called. Instead we probably should just call View::sync_changes in write_all_impl when we select a non-focused view.

The same bug exists for anything that edits multiple views (almost always the LSP) since only the focused view can be assumed to be up to date, so we need to fix those other instances too. For example in commands/lsp.rs again by inserting appropriate View::sync_changes calls when selecting a non-focused view.

pascalkuthe avatar Apr 14 '23 17:04 pascalkuthe

I ran into this as well (didn't have RUST_BACKTRACE set unfortunately):

thread 'main' panicked at 'Position 2335 is out of range for changeset len 1570!', helix-core/src/transaction.rs:399:13

I have autoformat enabled, and was editing a file that had changed outside of helix. I suspended helix, ran git checkout, and resumed helix. I'm not sure exactly what I did after that to trigger it.

rcorre avatar Apr 19 '23 17:04 rcorre

thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Char index out of bounds: char index 8759, Rope/RopeSlice char length 2489', /home/mcdoker18/.cargo/registry/src/github.com-1ecc6299db9ec823/ropey-1.6.0/src/rope.rs:690:41
stack backtrace:
   0: rust_begin_unwind
             at /rustc/8460ca823e8367a30dda430efda790588b8c84d3/library/std/src/panicking.rs:575:5
   1: core::panicking::panic_fmt
             at /rustc/8460ca823e8367a30dda430efda790588b8c84d3/library/core/src/panicking.rs:64:14
   2: core::result::unwrap_failed
             at /rustc/8460ca823e8367a30dda430efda790588b8c84d3/library/core/src/result.rs:1790:5
   3: helix_term::commands::lsp::compute_inlay_hints_for_all_views
   4: helix_term::ui::editor::EditorView::handle_idle_timeout
   5: <helix_term::ui::editor::EditorView as helix_term::compositor::Component>::handle_event
   6: helix_term::compositor::Compositor::handle_event
   7: helix_term::application::Application::handle_editor_event::{{closure}}
   8: hx::main_impl::{{closure}}
   9: tokio::runtime::context::BlockingRegionGuard::block_on
  10: tokio::runtime::scheduler::multi_thread::MultiThread::block_on
  11: tokio::runtime::runtime::Runtime::block_on
  12: hx::main

mcdoker18 avatar Apr 21 '23 09:04 mcdoker18

Helix Version: 23.05 on Linux : thread 'main' panicked at 'Position 4869 is out of range for changeset len 4204!', helix-core/src/transaction.rs:400:13

I think i was renaming a fn with LSP (with unsaved changes before it in file containing the fn and some other file using it. Also had a vertical split view showing both unsaved files, the one using the fn was on the left and the one containing the fn on the right).

It probably panicked on save with :w or right after hitting enter while renaming, not sure..

Relevant LSP related items from my config before panic :

[editor]
auto-pairs = false
auto-info = false
auto-format = false

[editor.lsp]
display-messages = true
display-signature-help-docs = false

Though these are unlikely the cause, judging by the description above.

EDIT: Nothing in helix.log file

zuixalias avatar Jun 27 '23 05:06 zuixalias

I've hit this one a few times over the past few days on 23.05, as far as I can tell, it happens when:

  • I use :write-all
  • In a language with an LSP/formatter
  • And the formatter has updates to apply

Specifically, I use goimports as my Go formatter, and sometimes that takes a second to run. Seems like there's some race between the LSP/formatter and file saving. IIRC, this was the root cause of an earlier panic as well, https://github.com/helix-editor/helix/issues/3459

It might also have happened on :write, but I'm not 100% sure of that.

bcspragu avatar Jun 27 '23 15:06 bcspragu

I described what causes this bug and how to fix it. I personally just haven't got the time to implement that myself

pascalkuthe avatar Jun 27 '23 15:06 pascalkuthe

This might be related but not sure if it's the same.

I was using 23.05 (b33516fb) and hit / and suddenly it crashed with a similar message. This has happened multiple times now.

I don't think I wrote :w nor :wa before it crashed, nor did I have any split views. It felt like I was just in insert mode, but I willl pay more attention next time

thread 'main' panicked at 'Positions [(6478, After), (6478, After), (6478, After), (6478, After), (6585, After), (6585, After), (6613, After), (6613, After), (14471, After), (14471, After)] are out of range for changeset len 15857!', /home/erik/Documents/GitHub/helix/helix-core/src/transaction.rs:413:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

I will clone the latest version and run with full backtrace and return if I find a reproducible case

EriKWDev avatar Jul 02 '23 08:07 EriKWDev

I described what causes this bug and how to fix it. I personally just haven't got the time to implement that myself

Apologies, that'll teach me to refresh my memory of a thread before commenting.

A semi-related new piece of data: I hit what I believe is this crash again when running :wa, and one of my files was completely blank afterwards. It seems like something about the race condition saved a blank file instead of the expected contents. It was a TypeScript file with typescript-language-server enabled.

A crash is disruptive, but completely erasing a file is much worse. I was able to recover ~80% of my work by grepping my drive for a known string, but it definitely wasn't a pleasant experience.

It sounds like the complexity of this issue is above my Rust/Helix paygrade, but I'll take a look and see if I can contribute a patch here, as this issue has probably lost me 10+ hours of work this past month.

bcspragu avatar Jul 03 '23 14:07 bcspragu