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

[transaction.rs] Positions [(18, Before)] are out of range for changeset len 0! #11291

Closed
hakan-demirli opened this issue Jul 24, 2024 · 7 comments · Fixed by #11304
Closed
Labels
C-bug Category: This is a bug

Comments

@hakan-demirli
Copy link

Summary

Crash on double tab.

Reproduction Steps

I can not consistently reproduce the issue. Out of nowhere, it crashes when I press tab quickly to traverse the suggestion list.

Helix log

❯ RUST_BACKTRACE=full hx src/main/scala/Wood.scala:1:1
thread 'main' panicked at helix-core/src/transaction.rs:483:9:
Positions [(18, Before)] are out of range for changeset len 0!
stack backtrace:
   0:     0x55e6da038637 - <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt::hee93a2ee0c2227c5
   1:     0x55e6d9293dac - core::fmt::write::h7222d6bd37096f9b
   2:     0x55e6da03825e - std::io::Write::write_fmt::hf5bd345a4857fa1f
   3:     0x55e6da0383b4 - std::sys_common::backtrace::print::h87f644093c44c474
   4:     0x55e6da0495a7 - std::panicking::default_hook::{{closure}}::h54f259bcb3c96d54
   5:     0x55e6da0492a3 - std::panicking::default_hook::h4e7363e714c56f43
   6:     0x55e6da049b3f - std::panicking::rust_panic_with_hook::h4be469481b88451f
   7:     0x55e6da038c92 - std::panicking::begin_panic_handler::{{closure}}::h12fd05c40e3f67ec
   8:     0x55e6da038856 - std::sys_common::backtrace::__rust_end_short_backtrace::hda2ba2e5aeedda76
   9:     0x55e6da0496b4 - rust_begin_unwind
  10:     0x55e6d9197215 - core::panicking::panic_fmt::h993794e49879741f
  11:     0x55e6d95643c4 - helix_core::transaction::ChangeSet::map_pos::hcbeb3bec4ec3a509
  12:     0x55e6d9d60710 - helix_view::document::Document::apply_impl::h8311d1021f96912a
  13:     0x55e6d9d619ef - helix_view::document::Document::apply_inner::h46a7a85e03ad3885
  14:     0x55e6d9d629f1 - helix_view::document::Document::restore::h4a4033b4cf34933d
  15:     0x55e6d9a3d590 - helix_term::ui::completion::Completion::new::{{closure}}::h4e67616ce2765a7a
  16:     0x55e6d9a4042d - <helix_term::ui::menu::Menu<T> as helix_term::compositor::Component>::handle_event::hf7252388d79af434
  17:     0x55e6d9a41601 - <helix_term::ui::popup::Popup<T> as helix_term::compositor::Component>::handle_event::hd68cca0b7fb0f0c1
  18:     0x55e6d997b0b2 - <helix_term::ui::editor::EditorView as helix_term::compositor::Component>::handle_event::h34ca80907d5c6f1c
  19:     0x55e6d9c3a4da - helix_term::compositor::Compositor::handle_event::ha48156f43df64fb9
  20:     0x55e6d9dde4c2 - hx::main_impl::{{closure}}::haf4e3af81e7e0795
  21:     0x55e6d9ddbc0f - tokio::runtime::park::CachedParkThread::block_on::hb91abd0ba48674e2
  22:     0x55e6d9e5b7a3 - tokio::runtime::runtime::Runtime::block_on::he501342a50889d29
  23:     0x55e6d9e7c7f3 - hx::main::hc28086a1210524ff
  24:     0x55e6d9e3c663 - std::sys_common::backtrace::__rust_begin_short_backtrace::ha171fbc5b7f8d5f7
  25:     0x55e6d9e6e73d - std::rt::lang_start::{{closure}}::hdac3ecca12f2d820
  26:     0x55e6da02b0f5 - std::rt::lang_start_internal::h5dd16626185ad212
  27:     0x55e6d9e7c8e5 - main
  28:     0x7f7ef7d1114e - __libc_start_call_main
  29:     0x7f7ef7d11209 - __libc_start_main@@GLIBC_2.34
  30:     0x55e6d9213e45 - _start
  31:                0x0 - <unknown>

helix.log

Platform

Linux (NixOS)

Terminal Emulator

kitty 0.35.2 from nixpkgs unstable

Installation Method

nixpkgs (but commit rev is overriden)

Helix Version

24.7 (4c18355)

@hakan-demirli hakan-demirli added the C-bug Category: This is a bug label Jul 24, 2024
@hakan-demirli
Copy link
Author

The issue was not present on b0cf86d.

@dxtr85
Copy link
Contributor

dxtr85 commented Jul 24, 2024

I have also stumbled upon this panic while editing some Rust code and trying to select an item from context menu which appears when you want to instantiate a new variable with a value of SpecificType:: ...
Same hx version as mentioned in OP.

@ondrej-ivanko
Copy link

ondrej-ivanko commented Jul 24, 2024

I think it was introduced in this commit #10559. It's the only significant update which also makes changes in lsp module. The autocompletion for lsp commands does not work anymore, when the completion happens at the end of the line and there is only new line (\n) at the end of the line. The issue was not present on 22/7 when I recompiled and used helix. I do that every day. I uploaded relevant parts of the helix log.

EDIT: Actually it seem to be rather random. It seems to happen also when autocompleting in between other characters, but sometimes it brakes, and sometimes it doesn't.

EDIT2: Can confirm issue is gone, when dropping above mentioned commit. Looks related to #11269
helix.log

@the-mikedavis
Copy link
Member

Is there a reliable way to reproduce this even if it takes a few tries to trigger the panic? For example a Rust file or project and a specific edit that can cause it. I would be surprised if #10559 caused this: it updates Document::apply in a way that could lead to a stacktrace like this but I've been running those changes for months and haven't seen this panic. The update to the LSP commands module changes how a command accesses the viewport's anchor. It shouldn't be a functional/behavior change.

@ondrej-ivanko
Copy link

@the-mikedavis I will try to come up with some reproducible example. There was either an inherent error in the lsp servers I'm using which were exposed by #10559 or the commit is the culprit. There doesn't seem to be other explanation I can think of. I tested behaviour on fresh helix compiles on commits from 22/7 and 23/7.

@GregorLohaus
Copy link

GregorLohaus commented Jul 24, 2024

I have a reproducable example:
Os: Debian 12
Version: Linux version 6.1.0-22-amd64 ([email protected]) (gcc-12 (Debian 12.2.0-14) 12.2.0, GNU ld (GNU Binutils for Debian) 2.40) #1 SMP PREEMPT_DYNAMIC Debian 6.1.94-1 (2024-06-21)
Terminal: https://github.com/wez/wezterm.git (7e8fdc118) build without wayland support
Language Server: https://github.com/LuaLS/lua-language-server (7d06e557)
Helix: 4c18355
Code:

local wezterm = require 'wezterm';
local config = wezterm.config_builder();
local helix_binary_name = '/home/anon/Src/helix/target/release/hx';
local yazi_binaty_name = '/home/anon/.cargo/bin/yazi';
---@param win table
---@param t string
local function toast(win,t)
        win:toast_notification(
          'Toast from WindowID: ' .. win:window_id(),
          t,
          nil,
          4000
        )
end
config.color_scheme = 'Gruvbox dark, hard (base16)';
config.keys = {
  {
    key = 'x',
    mods = 'ALT',
    action = wezterm.action.CloseCurrentPane { confirm = true }
  },
  {
    key = 'h',
    mods = 'ALT',
    action = wezterm.action.ActivatePaneDirection 'Left',
  },
  {
    key = 'j',
    mods = 'ALT',
    action = wezterm.action.ActivatePaneDirection 'Down',
  },
  {
    key = 'k',
    mods = 'ALT',
    action = wezterm.action.ActivatePaneDirection 'Up',
  },
  {
    key = 'l',
    mods = 'ALT',
    action = wezterm.action.ActivatePaneDirection 'Right',
  },
  {
    key = 'y',
    mods = 'ALT|SHIFT',
    action = wezterm.action_callback(function(win,pane)
          toast(win,pane:get_foreground_process_name())
    end)
  },
  {
    key = 'y',
    mods = 'ALT',
    action = wezterm.action_callback(function(win,pane)
          if pane:get_foreground_process_name() == helix_binary_name then
          end
    end)
  }
};
return config;

-- Useful functions: 
-- pane:get_foreground_process_name()
-- pane:pane_id()

helix inputs to reproduce: 53 gg gl a (Return) to (Tab)
stack trace:

thread 'main' panicked at helix-core/src/transaction.rs:483:9:
Positions [(76, Before)] are out of range for changeset len 0!
stack backtrace:
   0: rust_begin_unwind
             at /rustc/79e9716c980570bfd1f666e3b16ac583f0168962/library/std/src/panicking.rs:597:5
   1: core::panicking::panic_fmt
             at /rustc/79e9716c980570bfd1f666e3b16ac583f0168962/library/core/src/panicking.rs:72:14
   2: helix_core::transaction::ChangeSet::map_pos
   3: helix_view::document::Document::apply_impl
   4: helix_view::document::Document::apply_inner
   5: helix_view::document::Document::restore
   6: helix_term::ui::completion::Completion::new::{{closure}}
   7: <helix_term::ui::menu::Menu<T> as helix_term::compositor::Component>::handle_event
   8: <helix_term::ui::popup::Popup<T> as helix_term::compositor::Component>::handle_event
   9: <helix_term::ui::editor::EditorView as helix_term::compositor::Component>::handle_event
  10: helix_term::compositor::Compositor::handle_event
  11: helix_term::application::Application::run::{{closure}}
  12: tokio::runtime::park::CachedParkThread::block_on
  13: hx::main
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

@the-mikedavis
Copy link
Member

I can reproduce it, thanks for the concrete case!

The issue is we update the view positions in a block that we might run if we're applying a transaction with an empty changeset:

for view_data in self.view_data.values_mut() {
view_data.view_position.anchor = transaction
.changes()
.map_pos(view_data.view_position.anchor, Assoc::Before);
}

(introduced in #10559). The reason I don't see this locally is that I'm also running #9801 which refactors Document::apply_impl too. Fixing this is a good opportunity to land some of the changes from that PR that refactor and clean up that block (which have been tricky to rebase) @pascalkuthe.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants