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

Helix 23.05 (d52b7903) freezes editing markdown #7689

Closed
davidthewatson opened this issue Jul 20, 2023 · 17 comments · Fixed by #7737
Closed

Helix 23.05 (d52b7903) freezes editing markdown #7689

davidthewatson opened this issue Jul 20, 2023 · 17 comments · Fixed by #7737
Labels
C-bug Category: This is a bug upstream

Comments

@davidthewatson
Copy link

Summary

I've been running the git release of Helix on arch for a couple years now. My current install is on Garuda Linux:

     ╭─watson@acer in ~ took 8ms
     ╰─λ pamac info helix-git
    Name                  : helix-git
    Version               : 23.05.242.gd52b79037-1
    Description           : A text editor written in rust
    URL                   : https://helix-editor.com
    Licenses              : Unknown
    Repository            : chaotic-aur
    Installed Size        : 144.7 MB
    Groups                : --
    Depends On            : --
    Optional Dependencies : --
    Required By           : --
    Optional For          : --
    Provides              : hx
    Replaces              : --
    Conflicts With        : helix
    Packager              : UFSCar HPC Builder <[email protected]>
    Build Date            : Wed 19 Jul 2023 08:09:51 PM EDT
    Install Date          : Thu 20 Jul 2023 05:07:40 AM EDT
    Install Reason        : Explicitly installed
    Validated By          : Signature
    Backup files          : --

I've been editing markdown all day, every day for a while and helix started freezing recently where all keystroke processing ceases. Nothing seems to release it, esc, colon, etc.

I've taken to pgrep hx follwed by pkill hx at the command line to resolve and restart helix.

Reproduction Steps

I tried this:

  1. hx
  2. edit files with md extension
  3. wait for freeze within a few minutes

I expected this to happen:

No frezee.

Instead, this happened:

Freeze.

Helix log

I'll provide the verbose log when I get it.

~/.cache/helix/helix.log
 ╭─watson@acer in ~ took 16ms
 ╰─λ cat ~/.cache/helix/helix.log
File: /home/watson/.cache/helix/helix.log
2023-07-13T08:32:07.058 helix_view::editor [ERROR] Failed to initialize the language servers for `source.md` { cannot find binary path }
2023-07-13T08:32:56.575 helix_view::editor [ERROR] Failed to initialize the language servers for `source.md` { cannot find binary path }
2023-07-13T08:36:45.385 helix_view::editor [ERROR] Failed to initialize the language servers for `source.md` { cannot find binary path }
2023-07-19T12:19:55.671 helix_view::editor [ERROR] Failed to initialize the language servers for `source.bash` { cannot find binary path }
2023-07-20T03:56:06.337 helix_view::editor [ERROR] Failed to initialize the language servers for `source.md` { cannot find binary path }
2023-07-20T05:04:27.109 helix_view::editor [ERROR] Failed to initialize the language servers for `source.md` { cannot find binary path }
2023-07-20T05:24:50.686 helix_view::editor [ERROR] Failed to initialize the language servers for `source.md` { cannot find binary path }
2023-07-20T05:31:15.038 helix_view::editor [ERROR] Failed to initialize the language servers for `source.css` { cannot find binary path }
2023-07-20T05:34:19.761 helix_view::editor [ERROR] Failed to initialize the language servers for `source.md` { cannot find binary path }

Platform

Gaurda Linux (arch)

Terminal Emulator

fish on alacritty

Helix Version

helix 23.05 (d52b790)

@davidthewatson davidthewatson added the C-bug Category: This is a bug label Jul 20, 2023
@pascalkuthe
Copy link
Member

Please provide the file that caused this (or even better a mini.ized reproduction case). Otherwise this is not actionable

@pascalkuthe pascalkuthe added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Jul 20, 2023
@davidthewatson
Copy link
Author

Thanks! Sounds good but it'll take some work to anonymize or reduce. In the meantime, here's my reproduction case with hope that this may trigger someone:

  1. RUST_BACKTRACE=1 hx -vv .
  2. choose the MD file by heuristic search followed by ENTER: 3-4 characters in my directory tree and file case.
  3. type: /awa
  4. type: jjjj
  5. type: llllll
  6. FREEZE!

I suspect this will work with HTML too though I'm not sure of the root cause given the relationship of key events to the text file's size and implied parse complexity of its tokens.

I'll try to get a minimal reproduction file now that I know it doesn't take much to freeze helix.

@davidthewatson
Copy link
Author

I believe this is the relevant log:

z|lzo|lzma|lz4|lz|tbz2|tgz|sz|lzo|zst|bz2|gz|zip)$", opts: GlobOptions { case_insensitive: false, literal_separator: true, backslash_escape: true }, tokens: Tokens([ZeroOrMore, Literal('
.'), Alternates([Tokens([Literal('c'), Literal('a'), Literal('b')]), Tokens([Literal('r'), Literal('a'), Literal('r')]), Tokens([Literal('7'), Literal('z')]), Tokens([Literal('x'), Liter
al('z')]), Tokens([Literal('Z')]), Tokens([Literal('z')]), Tokens([Literal('l'), Literal('z'), Literal('o')]), Tokens([Literal('l'), Literal('z'), Literal('m'), Literal('a')]), Tokens([L
iteral('l'), Literal('z'), Literal('4')]), Tokens([Literal('l'), Literal('z')]), Tokens([Literal('t'), Literal('b'), Literal('z'), Literal('2')]), Tokens([Literal('t'), Literal('g'), Lit
eral('z')]), Tokens([Literal('s'), Literal('z')]), Tokens([Literal('l'), Literal('z'), Literal('o')]), Tokens([Literal('z'), Literal('s'), Literal('t')]), Tokens([Literal('b'), Literal('
z'), Literal('2')]), Tokens([Literal('g'), Literal('z')]), Tokens([Literal('z'), Literal('i'), Literal('p')])])]) }
2023-07-20T06:05:41.251 globset [DEBUG] built glob set; 0 literals, 0 basenames, 0 extensions, 0 prefixes, 0 suffixes, 0 required extensions, 1 regexes
2023-07-20T06:05:41.252 globset [DEBUG] glob converted to regex: Glob { glob: "**/.DS_Store?", re: "(?-u)^(?:/?|.*/)\\.DS_Store[^/]$", opts: GlobOptions { case_insensitive: false, litera
l_separator: true, backslash_escape: true }, tokens: Tokens([RecursivePrefix, Literal('.'), Literal('D'), Literal('S'), Literal('_'), Literal('S'), Literal('t'), Literal('o'), Literal('r
'), Literal('e'), Any]) }
2023-07-20T06:05:41.252 globset [DEBUG] glob converted to regex: Glob { glob: "**/._*", re: "(?-u)^(?:/?|.*/)\\._[^/]*$", opts: GlobOptions { case_insensitive: false, literal_separator:
true, backslash_escape: true }, tokens: Tokens([RecursivePrefix, Literal('.'), Literal('_'), ZeroOrMore]) }
2023-07-20T06:05:41.252 globset [DEBUG] built glob set; 5 literals, 3 basenames, 1 extensions, 0 prefixes, 0 suffixes, 0 required extensions, 2 regexes
2023-07-20T06:05:41.252 ignore::walk [DEBUG] ignoring ./.git: Ignore(IgnoreMatch(Hidden))
2023-07-20T06:05:41.252 ignore::walk [DEBUG] ignoring ./.gitignore: Ignore(IgnoreMatch(Hidden))
2023-07-20T06:05:41.254 helix_term::ui [DEBUG] file_picker init 4.260672ms
2023-07-20T06:05:41.255 helix_tui::backend::crossterm [DEBUG] The keyboard enhancement protocol is not supported in this terminal (checked in 511.186µs)```

@pascalkuthe
Copy link
Member

I can not reporduce with those steps (I used the helix changelog). I edit MD files very frequently so if this was easily reproducible I would have noticed. You need to provide a file. If you can minimize that would be nice but even a non-minimal file would be good.

@Zykino
Copy link

Zykino commented Jul 21, 2023

Hello, I got it when modifying HTML inside of a markdown file. It makes a core spin at 100% and I have to SIGKILL (-9) as SIGTERM (-15) have no effect. (Just updated my helix install to check if it was resolved in the meantime.)

My steps to reproduces ain’t minimal, sorry:

  1. git clone https://github.com/elipapa/markdown-cv.git
  2. cd markdown-cv
  3. mkdir cv
  4. mv index.md cv
  5. hx
  6. "Space f" and open the index file
  7. Edit the html paragraph near the top. (Add one or two lines/balise, modify text, go back and forth between insert and normal mode)

I did not had a crash on my little testing if I was on the project’s parent directory or if I opened the file directly with hx cv/index.md.

May be of some importance, or not, my env is as follow:
Linux zykino-pop-os 6.2.6-76060206-generic #202303130630168547333822.04~995127e SMP PREEMPT_DYNAMIC Tue M x86_64 x86_64 x86_64 GNU/Linux
alacritty 0.11.0
zellij 0.37.2
fish, version 3.3.1
helix 23.05 (457b389)

@davidthewatson
Copy link
Author

davidthewatson commented Jul 21, 2023

Thanks @Zykino for your effort.

I was hoping that someone else would see the same pattern.

I didn't have time to narrow my case but the reproduction in your git repo is almost identical: CV in markdown with embedded HTML.

I actually went looking back to the conversation between Gruber and Swartz around the creation of markdown twenty years ago, but I could not find the origin of the idea superset of HTML despite numerous deep conversations over the years.

I also pointed my package manager at a slightly different AUR and installed a more stable (not tip of the trunk) version and indeed, the regression disappears.

Name                  : helix
Version               : 23.05-1
Description           : A post-modern modal text editor
URL                   : https://helix-editor.com
Licenses              : MPL2
Repository            : extra
Installed Size        : 132.8 MB
Groups                : --
Depends On            : gcc-libs glibc hicolor-icon-theme
Optional Dependencies : bash-language-server: for Bash language support [Installed]
                        clang: for C/C++ language support [Installed]
                        dart: for Dart language support [Installed]
                        elvish: for elvish language support [Installed]
                        gopls: for Go language support [Installed]
                        haskell-language-server: for Haskell language support [Installed]
                        julia: for Julia language support [Installed]
                        lua-language-server: for Lua language support [Installed]
                        python-lsp-server: for Python language support [Installed]
                        r: for R and rmarkdown language support [Installed]
                        racket: for racket language support [Installed]
                        rust-analyzer: for Rust language support [Installed]
                        taplo: for TOML language support [Installed]
                        texlab: for LaTeX language support [Installed]
                        typescript-language-server: for jsx, tsx, typescript language support [Installed]
                        vscode-css-languageserver: for CSS and SCSS support [Installed]
                        vscode-html-languageserver: for HTML language support [Installed]
                        yaml-language-server: for YAML language support [Installed]
                        zls: for Zig language support [Installed]
Required By           : --
Optional For          : --
Provides              : --
Replaces              : --
Conflicts With        : --
Packager              : Orhun Parmaksız <[email protected]>
Build Date            : Thu 18 May 2023 01:46:55 PM EDT
Install Date          : Thu 20 Jul 2023 06:47:01 PM EDT
Install Reason        : Explicitly installed
Validated By          : Signature
Backup files          : --

I did try the reproduction steps with the repo on this install of hx and indeed, no crash. I'll try the other version today when I get a chance.

@pascalkuthe Thanks!

@pascalkuthe
Copy link
Member

thanks for the reproduction case @Zykino I was able to reproduce and trace this back.

This is an upstream bug in tree-sitter (or maybe the grammar but I actually doubt that). It seems that ts_node__prev_sibling gets stuck in an infinite loop if there are no previous children. The reason you only observe this in the latest master is that #7242 now also checks TS node siblings when rendering matching bracket highlights.

I checked and there is nothing wrong on our end here, TS just completely freezes up at some point.

@pascalkuthe pascalkuthe added upstream and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 21, 2023
@pascalkuthe
Copy link
Member

pascalkuthe commented Jul 21, 2023

minimal reproduction case:

Select the fist " in a md file that only contains the following HTML:

<x>
<""></x>

This freeze does not occur in an HTML file with the same content. Seems to be related to the fact that this is an injection Despite the looks this freeze actually also occurs when the HTML injections are disabled. The bracket matching currently doesn't account for injections (something that needs to be fixed in the future too)

@Zykino
Copy link

Zykino commented Jul 21, 2023

Wow thanks for the quick feedback :)
Did you open an issue upstream? Can you link it? So we can also follow the resolution on their side too.

@pascalkuthe
Copy link
Member

pascalkuthe commented Jul 21, 2023

I am still working on a minimal reproduction case for that. The current minimal reproduction case involves helix and is too complex to report upstream. We would have to write a small rust program that uses tree sitter and libloading to parse the snippet I posted above into a syntax tree and then call prev_siblings at the correct position. I didn't get around yo that yet

@yo-main
Copy link
Contributor

yo-main commented Jul 22, 2023

Encountered the same kind of freeze while editing some python through markdown. Would that be related ?
Helix freeze upon walking down the the 5 in the extract below and I tracked it down to something happening while matching bracket as well.

(remove the backslash, I couldn't format it correctly 😄 )

this is a code example in python

\`\`\`python
>>> len(data1)
577563
>>> data1[0]
{'route': '3', 'date': '01/01/2001', 'daytype': 'U', 'rides': 7354}
>>>

\`\`\`

@pascalkuthe
Copy link
Member

reported upstream in tree-sitter/tree-sitter#2421, reason seems to be a token of length 0 generated by the markdown grammar when HTML spans multiple lines.

@pascalkuthe
Copy link
Member

CC @MDeiml the freeze seems to be caused by block_continuation being an empty token. There are a lot of issues about TS freezing on empty tokens during paring (altough mine seems to be the first one about a case after parsing). Can we workaround this on the grammar by somehow not emitting empty tokens/nodes?

@MDeiml
Copy link
Contributor

MDeiml commented Jul 24, 2023

That should be possible for block continuations because their job I just to "subtract" from the inline part. Obviously subtracting something empty is not necessary. There are other empty nodes where this wouldn't be possible though, but maybe this is really just about the block continuations so I'll try.

@MDeiml
Copy link
Contributor

MDeiml commented Jul 24, 2023

I checked it out, but sadly I was wrong. These computations are somewhat hacky, but not sure if theres an easy way to do this.

The problem is that the state of the external scanner can only change if it emits a token. In the current implementation this is necessary sometimes without consuming any actual input. I think it's still possible to avoid empty block continuations (and maybe make stuff more efficient and clear in the process), but it's gonna take some work.

@pascalkuthe
Copy link
Member

pascalkuthe commented Jul 25, 2023

The upstream issue has been closed thanks to a quick fix by @the-mikedavis so there shouldn't be changes to the grammar necessary @MDeiml

@MDeiml
Copy link
Contributor

MDeiml commented Jul 25, 2023

Guess that should be "shouldn't"? :)

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 upstream
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants