Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
bcspragu opened this issue Apr 14, 2023 · 13 comments · Fixed by #9173
Closed
Labels
A-helix-term Area: Helix term improvements C-bug Category: This is a bug E-has-instructions Call for participation: Has instructions for fixing the issue and opening a PR

Comments

@bcspragu
Copy link
Contributor

bcspragu commented Apr 14, 2023

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 (3cf0372)

@bcspragu bcspragu added the C-bug Category: This is a bug label Apr 14, 2023
@bcspragu bcspragu changed the title 'Position ... is out of range for changeset len ....!' panic on ~23.03 'Position ... is out of range for changeset len ....!' panic on ~23.03 when saving a file Apr 14, 2023
@trink
Copy link
Contributor

trink commented Apr 14, 2023

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

@pascalkuthe
Copy link
Member

@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

@trink
Copy link
Contributor

trink commented Apr 14, 2023

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?

@pascalkuthe

This comment was marked as outdated.

@bcspragu

This comment was marked as outdated.

@pascalkuthe
Copy link
Member

@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 pascalkuthe added the E-has-instructions Call for participation: Has instructions for fixing the issue and opening a PR label Apr 14, 2023
@kirawi kirawi added the A-helix-term Area: Helix term improvements label Apr 17, 2023
@rcorre

This comment was marked as duplicate.

@mcdoker18

This comment was marked as resolved.

@zuixalias
Copy link

zuixalias commented Jun 27, 2023

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

@bcspragu
Copy link
Contributor Author

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, #3459

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

@pascalkuthe
Copy link
Member

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

@EriKWDev
Copy link

EriKWDev commented Jul 2, 2023

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

@bcspragu
Copy link
Contributor Author

bcspragu commented Jul 3, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-helix-term Area: Helix term improvements C-bug Category: This is a bug E-has-instructions Call for participation: Has instructions for fixing the issue and opening a PR
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants