helix-editor / helix

A post-modern modal text editor.
https://helix-editor.com
Mozilla Public License 2.0
33.07k stars 2.46k forks source link

Freezes with 100% CPU when trying to open a specific Nix file #4862

Closed max-privatevoid closed 1 year ago

max-privatevoid commented 1 year ago

Summary

Probably related to https://github.com/helix-editor/helix/issues/2997

Reproduction Steps

Helix log

No response

Platform

Linux

Terminal Emulator

Tilix 1.9.5

Helix Version

helix 22.08.1

the-mikedavis commented 1 year ago

This is probably from the tree-sitter-bash injections within strings. Tree-sitter-bash has an external scanner that looks like other problematic scanners we've seen (https://github.com/helix-editor/helix/issues/3504, https://github.com/helix-editor/helix/issues/3376, https://github.com/helix-editor/helix/issues/2979), and looking at the source now, it looks like there is the usual workaround in place which prevents scanning if in error recovery: https://github.com/tree-sitter/tree-sitter-bash/blob/77cf8a7cab8904baf1a721762e012644ac1d4c7b/src/scanner.cc#L171-L191. We can probably fix this by updating tree-sitter-bash

EDIT: we're already on a version of tree-sitter-bash with that scanner fix. It looks like updating tree-sitter-bash isn't enough to fix this.

max-privatevoid commented 1 year ago

The "fix" also works in strings that are used as derivation names, or even when renaming variables. Even deleting things from the comment near the end makes the file openable. I don't think it's related to bash TS.

the-mikedavis commented 1 year ago

Looking at a flamegraph for this, it looks like we're stuck on tree-sitter's ts_parser__recover function which is the error recovery routine that caused problems in other languages. On the revision that we use (and master), tree-sitter-nix's scanner is mitigating the error recovery problem: https://github.com/cstrahan/tree-sitter-nix/blob/1b69cf1fa92366eefbe6863c184e5d2ece5f187d/src/scanner.c#L180-L190

If I comment out this block in nix injections then the problem goes away: https://github.com/helix-editor/helix/blob/590a628460c8fa8a4e101541d67289fa2fac95b5/runtime/queries/nix/injections.scm#L21-L28

The block I was pointing to in tree-sitter-bash isn't preventing scanning in error recovery, it's just part of the scanner 😅. ~Adding the usual change to the scanner that checks if we're in error recovery and returns false seems to fix this. I'll fork tree-sitter-bash and make a PR.~ Actually that doesn't fix it either. Something else must be keeping us in recovery mode.

Very strange that deleting from the comment removes the problem 🤔

the-mikedavis commented 1 year ago

It looks like this was a bug in tree-sitter itself and was fixed in https://github.com/tree-sitter/tree-sitter/pull/1952 (also see https://github.com/cstrahan/tree-sitter-nix/issues/34). Running from that commit fixes this and the other linked issues for me. Once tree-sitter cuts a new version we should be able to upgrade and close this.

(If you're building from source and want to use that change before it is published and updated within Helix, you can add a patch to the Cargo.toml in the Helix repository root:)

[patch.crates-io]
tree-sitter = { git = "https://github.com/tree-sitter/tree-sitter", rev = "e021d6e9799903312dfa3a1ed39e160008fe52ac" }
archseer commented 1 year ago

We could switch to a git dependency here too since tree-sitter hasn't been updated in 6 months. neovim did so too https://github.com/neovim/neovim/pull/21858