frequent crashing especially when dealing with function contexts #53
Comments
|
sorry to hear that. can you provide more details? like the exact files you set up? it should also be possible to pull up the vs code output panel and select millet to see details about the crashes. could you share the backtrack? |
|
|
I set up a very basic structure with main.mlb millet.toml main.sml 
|
|
ok i think im able to reproduce. it seems easier to reproduce with autosave on. with the following basic project structure: .vscode/settings.json{
"millet.server.diagnostics.moreInfoHint.enable": false,
"millet.format.engine": "none",
"files.autoSave": "afterDelay",
"files.autoSaveDelay": 100
}main.mlbmillet.tomlversion = 1and on the latest main (d965b01), with the following extra diff applied for debugging: diff --git a/crates/lang-srv/src/helpers.rs b/crates/lang-srv/src/helpers.rs
index 90f5b4bb..9ed066bf 100644
--- a/crates/lang-srv/src/helpers.rs
+++ b/crates/lang-srv/src/helpers.rs
@@ -41,6 +41,7 @@ pub(crate) fn apply_changes(
contents: &mut String,
mut content_changes: Vec<lsp_types::TextDocumentContentChangeEvent>,
) {
+ eprint!("apply_changes {contents:?} {content_changes:?} -> ");
// If at least one of the changes is a full document change, use the last of them as the starting
// point and ignore all previous changes.
let content_changes = match content_changes.iter().rposition(|change| change.range.is_none()) {
@@ -72,4 +73,5 @@ pub(crate) fn apply_changes(
contents.replace_range(std::ops::Range::<usize>::from(range), &change.text);
}
}
+ eprintln!("{contents:?}");
}
diff --git a/crates/lang-srv/src/notification.rs b/crates/lang-srv/src/notification.rs
index eee710c7..912b22f4 100644
--- a/crates/lang-srv/src/notification.rs
+++ b/crates/lang-srv/src/notification.rs
@@ -39,6 +39,7 @@ fn try_update_input(
|| change.typ == lsp_types::FileChangeType::CHANGED
{
let new_contents = cx.fs.read_to_string(path.as_path())?;
+ eprintln!("new contents {new_contents:?}");
entry.insert(new_contents);
} else if change.typ == lsp_types::FileChangeType::DELETED {
entry.remove();
@@ -53,11 +54,13 @@ fn try_update_input(
#[allow(clippy::too_many_lines)]
fn go(st: &mut St, mut n: Notification) -> ControlFlow<Result<()>, Notification> {
n = helpers::try_notif::<lsp_types::notification::DidChangeWatchedFiles, _>(n, |params| {
+ eprintln!("DidChangeWatchedFiles");
match &mut st.mode {
Mode::Root(root) => {
match try_update_input(&mut st.cx, &mut root.input, params.changes) {
Ok(_) => {
// TODO use path ids
+ eprintln!("try_update_input ok sources {:?}", root.input.sources);
}
Err(_) => root.input = st.cx.get_input(&root.path),
}
@@ -68,6 +71,7 @@ fn go(st: &mut St, mut n: Notification) -> ControlFlow<Result<()>, Notification>
Ok(())
})?;
n = helpers::try_notif::<lsp_types::notification::DidChangeTextDocument, _>(n, |params| {
+ eprintln!("DidChangeTextDocument");
let url = params.text_document.uri;
let path = convert::url_to_path_id(&st.cx.fs, &mut st.cx.paths, &url)?;
if let Mode::Root(root) = &mut st.mode {if i pull up main.sml and basically type random nonsense into it, i get this after a few seconds: it seems like what happens is:
i'm not sure how to fix this. unfortunately String::replace_range does not have a non-panicking equivalent, so the only way i can catch the error is to catch_unwind. and even once i do, it seems like the whole problem is that the contents of the file on disk are out of date. |
|
i released v0.14.3 to try to address the issue by just catching the unwind and basically doing nothing afterwards. not the greatest solution to be sure but it might help |
|
can you let me know if it helps? it seems to help for me in my testing. in that, it does still panic but doesn't crash the language server anymore |
|
I tested it out on mine and it looks good, nice work on identifying the specifics and a (temporary I suppose?) fix! |
|
yea i'd like for the 'fix' to be temporary. if i get a chance i'd like to see if i can get the opinion of more experienced language server developers, since (i think?) the issue is not with millet (or at least not only). i do know in general e.g. rust-analyzer does depend on being able to catch_unwind (so it is not compiled with panic=abort); one of the reasons they do this is to implement request cancellation (which millet does not implement), but an ancillary benefit may be that this case is already handled 'for free'. |
|
started a discussion to see if they have any insight rust-lang/rust-analyzer#16578 |
|
important excerpt from that discussion:
this seems like the better solution (as opposed to the current hack of catching the unwind). |
|
more correctly fixed in 99426ff by applying the advice to ignore open files when handling DidChangeWatchedFiles. though i guess i can leave the |
|
the better fix is released in v0.14.4 |
Environment
Issue and Steps to Reproduce
Setup a basic project structure with millet.toml, sources.mlb with a main.sml.
Start writing code, especially with functions
fun ....Get an error that says "extension crashed <3-5> times in the last 5 minutes, shutting down".
The text was updated successfully, but these errors were encountered: