Skip to content

Commit

Permalink
fix: fixed infinite retry. (#1180)
Browse files Browse the repository at this point in the history
fix: fixed infinite retry issue. Add checks for retry termination conditions and retry intervals

Signed-off-by: he1pa <[email protected]>
  • Loading branch information
He1pa authored Mar 29, 2024
1 parent c235886 commit 7021a73
Show file tree
Hide file tree
Showing 7 changed files with 63 additions and 39 deletions.
4 changes: 2 additions & 2 deletions kclvm/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion kclvm/tools/src/LSP/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ compiler_base_session = { path = "../../../../compiler_base/session" }
kclvm-query = { path = "../../../query" }
kclvm-span = { path = "../../../span" }

lsp-server = { version = "0.6.0", default-features = false }
lsp-server = { version = "0.7.0", default-features = false }
anyhow = { version = "1.0", default-features = false, features = ["std"] }
crossbeam-channel = { version = "0.5.7", default-features = false }
ra_ap_vfs = "0.0.149"
Expand Down
9 changes: 7 additions & 2 deletions kclvm/tools/src/LSP/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,14 @@ impl fmt::Display for LSPError {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
match self {
LSPError::Retry => write!(f, "{}", RETRY_REQUEST),
LSPError::FileIdNotFound(path) => write!(f, "Path {path} fileId not found"),
LSPError::FileIdNotFound(path) => {
write!(f, "Internal bug: Path {path} fileId not found")
}
LSPError::AnalysisDatabaseNotFound(path) => {
write!(f, "Path {path} AnalysisDatabase not found")
write!(
f,
"Internal bug: Path {path} analysisDatabase not found, maybe compile failed"
)
}
}
}
Expand Down
1 change: 0 additions & 1 deletion kclvm/tools/src/LSP/src/notification.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ impl LanguageServerState {
&mut self,
notification: lsp_server::Notification,
) -> anyhow::Result<()> {
self.log_message(format!("on notification {:?}", notification));
NotificationDispatcher::new(self, notification)
.on::<DidOpenTextDocument>(LanguageServerState::on_did_open_text_document)?
.on::<DidChangeTextDocument>(LanguageServerState::on_did_change_text_document)?
Expand Down
20 changes: 11 additions & 9 deletions kclvm/tools/src/LSP/src/request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ impl LanguageServerState {
request: lsp_server::Request,
request_received: Instant,
) -> anyhow::Result<()> {
log_message(format!("on request {:?}", request), &self.task_sender)?;
self.register_request(&request, request_received);

// If a shutdown was requested earlier, immediately respond with an error
Expand Down Expand Up @@ -106,11 +105,11 @@ impl LanguageServerSnapshot {
match file_id {
Some(id) => match self.db.read().get(&id) {
Some(db) => Ok(Arc::clone(db)),
None => Err(anyhow::anyhow!(LSPError::FileIdNotFound(path.clone()))),
None => Err(anyhow::anyhow!(LSPError::AnalysisDatabaseNotFound(
path.clone()
))),
},
None => Err(anyhow::anyhow!(LSPError::AnalysisDatabaseNotFound(
path.clone()
))),
None => Err(anyhow::anyhow!(LSPError::FileIdNotFound(path.clone()))),
}
}

Expand All @@ -122,12 +121,15 @@ impl LanguageServerSnapshot {
match self.vfs.try_read() {
Some(vfs) => match vfs.file_id(path) {
Some(file_id) => match self.db.try_read() {
Some(db) => Ok(db.get(&file_id).map(|db| Arc::clone(db))),
Some(db) => match db.get(&file_id) {
Some(db) => Ok(Some(Arc::clone(db))),
None => Err(anyhow::anyhow!(LSPError::AnalysisDatabaseNotFound(
path.clone()
))),
},
None => Ok(None),
},
None => Err(anyhow::anyhow!(LSPError::AnalysisDatabaseNotFound(
path.clone()
))),
None => Err(anyhow::anyhow!(LSPError::FileIdNotFound(path.clone()))),
},
None => Ok(None),
}
Expand Down
15 changes: 13 additions & 2 deletions kclvm/tools/src/LSP/src/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ use parking_lot::RwLock;
use ra_ap_vfs::{FileId, Vfs};
use std::collections::HashMap;
use std::sync::Mutex;
use std::thread;
use std::time::Duration;
use std::{sync::Arc, time::Instant};

pub(crate) type RequestHandler = fn(&mut LanguageServerState, lsp_server::Response);
Expand Down Expand Up @@ -256,8 +258,9 @@ impl LanguageServerState {
}));
}
Err(err) => {
db.remove(&file.file_id);
log_message(
format!("compile failed: {:?}", err.to_string()),
format!("Compile failed: {:?}", err.to_string()),
&sender,
);
}
Expand Down Expand Up @@ -289,7 +292,11 @@ impl LanguageServerState {
self.send(notification.into());
}
Task::Response(response) => self.respond(response)?,
Task::Retry(request) => self.on_request(request, request_received)?,
Task::Retry(req) if !self.is_completed(&req) => {
thread::sleep(Duration::from_millis(20));
self.on_request(req, request_received)?
}
Task::Retry(_) => (),
}
Ok(())
}
Expand Down Expand Up @@ -343,6 +350,10 @@ impl LanguageServerState {
);
self.send(not.into());
}

pub(crate) fn is_completed(&self, request: &lsp_server::Request) -> bool {
self.request_queue.incoming.is_completed(&request.id)
}
}

pub(crate) fn log_message(message: String, sender: &Sender<Task>) -> anyhow::Result<()> {
Expand Down
51 changes: 29 additions & 22 deletions kclvm/tools/src/LSP/src/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ use parking_lot::RwLockReadGuard;
use ra_ap_vfs::{FileId, Vfs};
use serde::{de::DeserializeOwned, Serialize};

use std::fs;
use std::path::Path;
use std::{fs, panic};

#[allow(unused)]
/// Deserializes a `T` from a json value.
Expand Down Expand Up @@ -82,27 +82,34 @@ pub(crate) fn compile_with_params(
let mut k_code_list = load_files_code_from_vfs(&files, vfs)?;
opt.k_code_list.append(&mut k_code_list);
}
// Parser
let sess = ParseSessionRef::default();
let mut program = load_program(sess.clone(), &files, Some(opt), params.module_cache)?.program;
// Resolver
let prog_scope = resolve_program_with_opts(
&mut program,
kclvm_sema::resolver::Options {
merge_program: false,
type_erasure: false,
..Default::default()
},
params.scope_cache,
);
// Please note that there is no global state cache at this stage.
let gs = GlobalState::default();
let gs = Namer::find_symbols(&program, gs);
let gs = AdvancedResolver::resolve_program(&program, gs, prog_scope.node_ty_map.clone());
// Merge parse diagnostic and resolve diagnostic
sess.append_diagnostic(prog_scope.handler.diagnostics.clone());
let diags = sess.1.borrow().diagnostics.clone();
Ok((program, diags, gs))
match panic::catch_unwind(move || {
// Parser
let sess = ParseSessionRef::default();
let mut program = load_program(sess.clone(), &files, Some(opt), params.module_cache)
.unwrap()
.program;
// Resolver
let prog_scope = resolve_program_with_opts(
&mut program,
kclvm_sema::resolver::Options {
merge_program: false,
type_erasure: false,
..Default::default()
},
params.scope_cache,
);
// Please note that there is no global state cache at this stage.
let gs = GlobalState::default();
let gs = Namer::find_symbols(&program, gs);
let gs = AdvancedResolver::resolve_program(&program, gs, prog_scope.node_ty_map.clone());
// Merge parse diagnostic and resolve diagnostic
sess.append_diagnostic(prog_scope.handler.diagnostics.clone());
let diags = sess.1.borrow().diagnostics.clone();
Ok((program, diags, gs))
}) {
Ok(res) => return res,
Err(e) => Err(anyhow::anyhow!("Compile failed: {:?}", e)),
}
}

/// Update text with TextDocumentContentChangeEvent param
Expand Down

0 comments on commit 7021a73

Please sign in to comment.