refactor(oxlint/lsp): lint tsgolint in the same thread#24670
Conversation
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
There was a problem hiding this comment.
Pull request overview
Refactors the tsgolint integration used by oxlint/LSP so diagnostics are read and processed in the calling thread instead of being handled via additional worker threads.
Changes:
- Removes the nested thread setup previously used to drain
tsgolintstdout and collect diagnostics. - Processes the
tsgolintmessage stream synchronously, then waits for the child process and returns diagnostics/errors.
Merging this PR will not alter performance
Comparing Footnotes
|
2d853b4 to
a32ed30
Compare
a32ed30 to
ca55327
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
crates/oxc_linter/src/tsgolint.rs:523
lint_sourceno longer waits on the spawnedtsgolintchild or checks its exit status. Callingkill()withoutwait()can still leave a zombie process on Unix, and failures (non-zero exit) can now be silently ignored even if stdout parsing succeeded.
// Kill the child process if it's still running to avoid zombie processes
let _ = child.kill();
diagnostics

This PR comes from different use cases, and I hope I did not have a wrong understanding about spawning child.
std::spawnintokioruntimeBecause the LSP is in a
tokioruntime, we should avoid spawning threads withstd, instead usingtokio::spawn.I try to work already with
tokio::spawnin theoxc_language_serverside with #23768So this will spawn the complete diagnostic path into one separate process. We already need to wait for the
tsgolintresult, so spawning one thread there is suboptimal(?)disable_directives_maplock for the whole thread spawnAsked Codex Terra 5.6 where a lock could be generated in #23768, and it found this long-lock.
tsgolint process still running
Some users did report that many tsgolint processes are still running, even if not typed. My first guess is these are zombie processes and can be safely killed.
I did not find the exact issue but oxc-project/oxc-vscode#264 should be good enough for a reference.