Skip to content

Commit d3594e2

Browse files
committed
fix(linter/plugins): handle error from destroyWorkspace
1 parent 255d7fc commit d3594e2

3 files changed

Lines changed: 44 additions & 4 deletions

File tree

apps/oxlint/src/js_plugins/external_linter.rs

Lines changed: 37 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,7 @@
1-
use std::sync::{Arc, atomic::Ordering, mpsc::channel};
1+
use std::{
2+
sync::{Arc, atomic::Ordering, mpsc::channel},
3+
time::Duration,
4+
};
25

36
use napi::{
47
Status,
@@ -326,8 +329,40 @@ fn wrap_create_workspace(cb: JsCreateWorkspaceCb) -> ExternalLinterCreateWorkspa
326329
}
327330

328331
/// Wrap `destroyWorkspace` JS callback as a normal Rust function.
332+
///
333+
/// The JS-side `destroyWorkspace` function is synchronous, but it's wrapped in a `ThreadsafeFunction`,
334+
/// so cannot be called synchronously. Use an `mpsc::channel` to wait for the result from JS side.
335+
///
336+
/// Uses a timeout to prevent indefinite blocking during shutdown, which can cause issues
337+
/// in multi-root workspace scenarios where multiple workspaces are being destroyed concurrently.
329338
fn wrap_destroy_workspace(cb: JsDestroyWorkspaceCb) -> ExternalLinterDestroyWorkspaceCb {
330339
Arc::new(Box::new(move |workspace_uri| {
331-
let _ = cb.call(workspace_uri, ThreadsafeFunctionCallMode::NonBlocking);
340+
let (tx, rx) = channel();
341+
342+
// Send data to JS
343+
let status = cb.call_with_return_value(
344+
workspace_uri,
345+
ThreadsafeFunctionCallMode::NonBlocking,
346+
move |result, _env| {
347+
// Ignore send errors - the receiver may have timed out
348+
let _ = tx.send(result);
349+
Ok(())
350+
},
351+
);
352+
353+
if status == Status::Ok {
354+
// Use a timeout to prevent blocking indefinitely during shutdown.
355+
// If JS side doesn't respond within the timeout, we proceed with shutdown anyway.
356+
match rx.recv_timeout(Duration::from_secs(5)) {
357+
// Destroying workspace succeeded
358+
Ok(Ok(())) => Ok(()),
359+
// `destroyWorkspace` threw an error
360+
Ok(Err(err)) => Err(format!("`destroyWorkspace` threw an error: {err}")),
361+
// Timeout or sender dropped - proceed with shutdown
362+
Err(_) => Ok(()),
363+
}
364+
} else {
365+
Err(format!("Failed to schedule `destroyWorkspace` callback: {status:?}"))
366+
}
332367
}))
333368
}

apps/oxlint/src/lsp/server_linter.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -301,7 +301,11 @@ impl ToolBuilder for ServerLinterBuilder {
301301
fn shutdown(&self, root_uri: &Uri) {
302302
// Destroy JS workspace
303303
if let Some(external_linter) = &self.external_linter {
304-
(external_linter.destroy_workspace)(root_uri.as_str().to_string());
304+
let res = (external_linter.destroy_workspace)(root_uri.as_str().to_string());
305+
306+
if let Err(err) = res {
307+
error!("Failed to destroy JS workspace:\n{err}\n");
308+
}
305309
}
306310
}
307311
}

crates/oxc_linter/src/external_linter.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,8 @@ use crate::{
1212
pub type ExternalLinterCreateWorkspaceCb =
1313
Arc<Box<dyn Fn(String) -> Result<(), String> + Send + Sync>>;
1414

15-
pub type ExternalLinterDestroyWorkspaceCb = Arc<Box<dyn Fn(String) + Send + Sync>>;
15+
pub type ExternalLinterDestroyWorkspaceCb =
16+
Arc<Box<dyn Fn(String) -> Result<(), String> + Send + Sync>>;
1617

1718
pub type ExternalLinterLoadPluginCb = Arc<
1819
Box<

0 commit comments

Comments
 (0)