From def9fba2b69a9f72bffd5656007106ea3af324a7 Mon Sep 17 00:00:00 2001 From: Josh Thomas Date: Thu, 15 May 2025 21:03:14 -0500 Subject: [PATCH] migrate all async tokio to server & swap to single-thread runtime (#149) --- Cargo.toml | 3 --- crates/djls-project/Cargo.toml | 1 - crates/djls-server/Cargo.toml | 6 +++--- crates/djls-server/src/documents.rs | 7 ------ crates/djls-server/src/lib.rs | 23 +++++++++++++++++++- crates/djls-server/src/queue.rs | 12 +++++++---- crates/djls-server/src/session.rs | 26 ++++------------------- crates/djls-templates/Cargo.toml | 1 - crates/djls-templates/src/error.rs | 32 ---------------------------- crates/djls-templates/src/lib.rs | 1 - crates/djls/Cargo.toml | 3 --- crates/djls/src/commands/serve.rs | 33 +++++++---------------------- 12 files changed, 45 insertions(+), 103 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index d87c87b..0fa898e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -13,15 +13,12 @@ djls-templates = { path = "crates/djls-templates" } anyhow = "1.0" async-trait = "0.1" pyo3 = "0.24" -pyo3-async-runtimes = "0.24" pyo3-build-config = "0.24" salsa = { git = "https://github.com/salsa-rs/salsa.git", rev = "7edce6e248f35c8114b4b021cdb474a3fb2813b3" } serde = { version = "1.0", features = ["derive"] } serde_json = "1.0" tempfile = "3.19" thiserror = "2.0" -tokio = { version = "1.42", features = ["full"] } -tower-lsp-server = { version = "0.21", features = ["proposed"] } [workspace.lints.clippy] pedantic = { level = "warn", priority = -1 } diff --git a/crates/djls-project/Cargo.toml b/crates/djls-project/Cargo.toml index 74d6212..024a65c 100644 --- a/crates/djls-project/Cargo.toml +++ b/crates/djls-project/Cargo.toml @@ -10,7 +10,6 @@ default = [] [dependencies] pyo3 = { workspace = true } salsa = { workspace = true } -tower-lsp-server = { workspace = true, features = ["proposed"] } which = "7.0.1" diff --git a/crates/djls-server/Cargo.toml b/crates/djls-server/Cargo.toml index db007e3..e9ef56a 100644 --- a/crates/djls-server/Cargo.toml +++ b/crates/djls-server/Cargo.toml @@ -17,10 +17,10 @@ pyo3 = { workspace = true } salsa = { workspace = true } serde = { workspace = true } serde_json = { workspace = true } -tokio = { workspace = true } -tower-lsp-server = { workspace = true } -percent-encoding = "2.3" +percent-encoding = "2.3.1" +tokio = { version = "1.45.0", features = ["full"] } +tower-lsp-server = { version = "0.21.1", features = ["proposed"] } [build-dependencies] djls-dev = { workspace = true } diff --git a/crates/djls-server/src/documents.rs b/crates/djls-server/src/documents.rs index 10de7ef..4725662 100644 --- a/crates/djls-server/src/documents.rs +++ b/crates/djls-server/src/documents.rs @@ -25,13 +25,6 @@ pub struct Store { } impl Store { - pub fn new() -> Self { - Self { - documents: HashMap::new(), - versions: HashMap::new(), - } - } - pub fn handle_did_open(&mut self, db: &dyn Database, params: &DidOpenTextDocumentParams) { let uri = params.text_document.uri.to_string(); let version = params.text_document.version; diff --git a/crates/djls-server/src/lib.rs b/crates/djls-server/src/lib.rs index 92ea569..99c0ee3 100644 --- a/crates/djls-server/src/lib.rs +++ b/crates/djls-server/src/lib.rs @@ -5,4 +5,25 @@ mod server; mod session; mod workspace; -pub use crate::server::DjangoLanguageServer; +use anyhow::Result; +use tower_lsp_server::LspService; +use tower_lsp_server::Server; + +use crate::server::DjangoLanguageServer; + +pub fn run() -> Result<()> { + let runtime = tokio::runtime::Builder::new_current_thread() + .enable_all() + .build()?; + + runtime.block_on(async { + let stdin = tokio::io::stdin(); + let stdout = tokio::io::stdout(); + + let (service, socket) = LspService::build(DjangoLanguageServer::new).finish(); + + Server::new(stdin, stdout, socket).serve(service).await; + + Ok(()) + }) +} diff --git a/crates/djls-server/src/queue.rs b/crates/djls-server/src/queue.rs index f1be168..b857958 100644 --- a/crates/djls-server/src/queue.rs +++ b/crates/djls-server/src/queue.rs @@ -19,7 +19,9 @@ use tokio::sync::oneshot; /// required for self-referential `async` blocks. /// - `Box`: Allocates the `Future` on the heap. /// - `dyn Future`: Type erasure - hides the specific concrete `Future` type. -/// - `+ Send`: Ensures the `Future` can be safely sent across threads. +/// - `+ Send`: Ensures the `Future` can be safely sent across threads and required +/// by the tower-lsp-server LSP server trait bounds, even in our single-threaded +/// runtime. type TaskFuture = Pin> + Send>>; /// Type alias for a type-erased, heap-allocated, Send-able closure that, @@ -34,7 +36,8 @@ type TaskFuture = Pin> + Send>>; /// arguments. /// - `-> TaskFuture`: Specifies that calling the closure produces the type-erased future. /// - `+ Send + 'static`: Ensures the closure itself can be safely sent across -/// threads and has a static lifetime (doesn't borrow short-lived data). +/// threads and has a static lifetime (doesn't borrow short-lived data). Required +/// for compatibility with our async runtime and LSP traits. type TaskClosure = Box TaskFuture + Send + 'static>; /// A simple asynchronous task queue for sequential execution. @@ -43,8 +46,9 @@ type TaskClosure = Box TaskFuture + Send + 'static>; /// to a dedicated worker task which executes them one at a time in the order /// they were received. This ensures sequential processing of background tasks. /// -/// The queue is cloneable (`Arc`-based internally), allowing multiple producers -/// to submit tasks concurrently. +/// The queue runs within our single-threaded runtime but maintains compatibility +/// with the Send+Sync requirements of the LSP. This provides the benefits of +/// simpler execution while maintaining the required trait bounds. /// /// Shutdown is handled gracefully when the last `Queue` instance is dropped. #[derive(Clone)] diff --git a/crates/djls-server/src/session.rs b/crates/djls-server/src/session.rs index cff7460..f825d64 100644 --- a/crates/djls-server/src/session.rs +++ b/crates/djls-server/src/session.rs @@ -19,6 +19,10 @@ pub struct Session { /// where we're using the `StorageHandle` to create a thread-safe handle that can be /// shared between threads. When we need to use it, we clone the handle to get a new reference. /// + /// This handle allows us to create database instances as needed. + /// Even though we're using a single-threaded runtime, we still need + /// this to be thread-safe because of LSP trait requirements. + /// /// Usage: /// ```rust,ignore /// // Use the StorageHandle in Session @@ -41,20 +45,6 @@ pub struct Session { } impl Session { - pub fn new(client_capabilities: ClientCapabilities) -> Self { - Self { - client_capabilities: Some(client_capabilities), - project: None, - documents: Store::new(), - settings: Settings::default(), - db_handle: StorageHandle::new(None), - } - } - - pub fn client_capabilities(&self) -> Option<&ClientCapabilities> { - self.client_capabilities.as_ref() - } - pub fn client_capabilities_mut(&mut self) -> &mut Option { &mut self.client_capabilities } @@ -83,14 +73,6 @@ impl Session { &mut self.settings } - /// Get the raw database handle from the session - /// - /// Note: In most cases, you'll want to use `db()` instead to get a usable - /// database instance directly. - pub fn db_handle(&self) -> &StorageHandle { - &self.db_handle - } - /// Get a database instance directly from the session /// /// This creates a usable database from the handle, which can be used diff --git a/crates/djls-templates/Cargo.toml b/crates/djls-templates/Cargo.toml index 6959e61..9e5a438 100644 --- a/crates/djls-templates/Cargo.toml +++ b/crates/djls-templates/Cargo.toml @@ -5,7 +5,6 @@ edition = "2021" [dependencies] anyhow = { workspace = true } -tower-lsp-server = { workspace = true } serde = { workspace = true } thiserror = { workspace = true } toml = "0.8" diff --git a/crates/djls-templates/src/error.rs b/crates/djls-templates/src/error.rs index c5bc4fd..6e55a54 100644 --- a/crates/djls-templates/src/error.rs +++ b/crates/djls-templates/src/error.rs @@ -1,6 +1,5 @@ use serde::Serialize; use thiserror::Error; -use tower_lsp_server::lsp_types; use crate::ast::AstError; use crate::ast::Span; @@ -52,17 +51,6 @@ impl TemplateError { } } - #[must_use] - pub fn severity(&self) -> lsp_types::DiagnosticSeverity { - match self { - TemplateError::Lexer(_) | TemplateError::Parser(_) => { - lsp_types::DiagnosticSeverity::ERROR - } - TemplateError::Validation(_) => lsp_types::DiagnosticSeverity::WARNING, - _ => lsp_types::DiagnosticSeverity::INFORMATION, - } - } - #[must_use] pub fn code(&self) -> &'static str { match self { @@ -75,26 +63,6 @@ impl TemplateError { } } -pub fn to_lsp_diagnostic(error: &TemplateError, _source: &str) -> lsp_types::Diagnostic { - let range = error.span().map_or_else(lsp_types::Range::default, |span| { - let start = lsp_types::Position::new(0, span.start()); - let end = lsp_types::Position::new(0, span.start() + span.length()); - lsp_types::Range::new(start, end) - }); - - lsp_types::Diagnostic { - range, - severity: Some(error.severity()), - code: Some(lsp_types::NumberOrString::String(error.code().to_string())), - code_description: None, - source: Some("djls-template".to_string()), - message: error.to_string(), - related_information: None, - tags: None, - data: None, - } -} - pub struct QuickFix { pub title: String, pub edit: String, diff --git a/crates/djls-templates/src/lib.rs b/crates/djls-templates/src/lib.rs index 9651a0f..4835056 100644 --- a/crates/djls-templates/src/lib.rs +++ b/crates/djls-templates/src/lib.rs @@ -6,7 +6,6 @@ mod tagspecs; mod tokens; use ast::Ast; -pub use error::to_lsp_diagnostic; pub use error::QuickFix; pub use error::TemplateError; use lexer::Lexer; diff --git a/crates/djls/Cargo.toml b/crates/djls/Cargo.toml index 79b6849..52c90ee 100644 --- a/crates/djls/Cargo.toml +++ b/crates/djls/Cargo.toml @@ -21,10 +21,7 @@ djls-server = { workspace = true } anyhow = { workspace = true } pyo3 = { workspace = true } -pyo3-async-runtimes = { workspace = true, features = ["tokio-runtime"] } serde_json = { workspace = true } -tokio = { workspace = true } -tower-lsp-server = { workspace = true } clap = { version = "4.5", features = ["derive"] } diff --git a/crates/djls/src/commands/serve.rs b/crates/djls/src/commands/serve.rs index ca88ed0..3388fdd 100644 --- a/crates/djls/src/commands/serve.rs +++ b/crates/djls/src/commands/serve.rs @@ -1,9 +1,6 @@ use anyhow::Result; use clap::Parser; use clap::ValueEnum; -use djls_server::DjangoLanguageServer; -use tower_lsp_server::LspService; -use tower_lsp_server::Server; use crate::args::Args; use crate::commands::Command; @@ -23,28 +20,14 @@ enum ConnectionType { impl Command for Serve { fn execute(&self, _args: &Args) -> Result { - let runtime = tokio::runtime::Builder::new_multi_thread() - .enable_all() - .build() - .unwrap(); + djls_server::run()?; - let exit_status = runtime.block_on(async { - let stdin = tokio::io::stdin(); - let stdout = tokio::io::stdout(); - - let (service, socket) = LspService::build(DjangoLanguageServer::new).finish(); - - Server::new(stdin, stdout, socket).serve(service).await; - - // Exit here instead of returning control to the `Cli`, for ... reasons? - // If we don't exit here, ~~~ something ~~~ goes on with PyO3 (I assume) - // or the Python entrypoint wrapper to indefinitely hang the CLI and keep - // the process running - Exit::success() - .with_message("Server completed successfully") - .process_exit() - }); - - Ok(exit_status) + // Exit here instead of returning control to the `Cli`, for ... reasons? + // If we don't exit here, ~~~ something ~~~ goes on with PyO3 (I assume) + // or the Python entrypoint wrapper to indefinitely hang the CLI and keep + // the process running + Exit::success() + .with_message("Server completed successfully") + .process_exit() } }