-
Notifications
You must be signed in to change notification settings - Fork 89
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
NLS crashes on recursive definitions #1875
Comments
nls
Crashes on recursive definitions
nls
Crashes on recursive definitions
This is a tough one, because while |
Yes, the background thread is doing it a little differently: its eval function ( Having said that, for me it looks like nls's protection against problems in eval is kicking in as expected: the background eval process crashes (and so we lose eval diagnostics, which is sad), but the main nls process keeps running. |
@uhlajs can you confirm that even after the crash, normal NLS functions are still working as expected (like goto definition or show information on hover)? If this is just the background evaluation process crashing, this is not ideal, but less critical. |
Negative (goto definition does not work), I am getting following message and there is no
I also tried to comment out the problematic line, then open editor and uncomment it. Here is result: $ pgrep nls
2863627
2863634
$ ps 2863627 2863634
PID TTY STAT TIME COMMAND
2863627 ? Ssl 0:00 nls
2863634 ? S 0:00 /nix/store/xjynz8qzh3abmsxa0yi0g8gh6am250dz-nickel-1.5.0-nls/bin/nls --main-server /tmp/nix-shell.
# Now I have uncommented and saved file.
$ pgrep nls
# Empty result |
Interesting. Here is what happens for me: nls.mp4On uncommenting that line, you can see a zombie process appear in htop (that's the crashed background process), but typechecking still works in the editor afterwards. Is there any way you can get a backtrace of the crashing nls? If nvim propagates its environment to nls, it should be enough to set Regarding trying to detect the recursion, the problem I'm having is that the environment is different each time we recurse so it isn't an obvious infinite loop to the interpreter. Probably for nls it's ok to bound the recursion depth? |
@jneem Here I am running similar commands: nls.mp4As you can see the I also tried to run latest code with #1878 changes, but the result is still the same. Unfortunately, I wasn't able to get backtrace with Content of #!/bin/bash
export RUST_BACKTRACE=1
/home/honza/git/github/nickel/target/debug/nls "$@" Full log:
|
Thanks for the core dumps, those are super helpful. One of them is an infinite recursion in |
Great, let me know if I can be more helpful here. |
Thank you @jneem! I can confirm that #1881 resolves issues with Unfortunately, the 2024-04-05.08-43-24.mp4Shall I open new issue or do you want to reuse this one? |
@uhlajs thanks for all the reporting 🙂 that's super useful! If you don't mind, I think opening a new issue is preferable for multiple reasons (searchability, self-containment, etc.), as NLS indeed doesn't crash anymore on recursive definitions. |
No problem -> moving the further discussion to #1882. |
Describe the bug
nls
crashes with stack overflow error when parsing nickel source with recursive definition.To Reproduce
Open file with following content in text editor with LSP support and running
nls
.I can reproduce it with lunarvim, but this should be easily reproduce with vanilla
neovim
or any other text editor.Expected behavior
Since this is a valid nickel source code, I would expect no
nls
crashes.Environment
Additional logs
The text was updated successfully, but these errors were encountered: