From 833a2a935a8b749de88241bd8db3fe3fd4577d6e Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Sat, 30 Jun 2018 14:01:27 +0300 Subject: [PATCH] Add concurrency module RLS is going to be a highly concurrent application, with three main concurrency sources: * user input/requests from the client editor * changes from the file system * internal parallelisation of CPU-heavy tasks Because developing, debugging and testing concurrent applications is hard, it makes sense to try to establish good concurrency patterns from the start. One concurrency anti-pattern is "fire & forget" spawning of background tasks. This is problematic: without the ability to check if the task is finished, you have to resort to busy-waiting in tests. `concurrency` module introduces a `ConcurrentJob` object, which is a handle to background job, which can be used to check if the job is running and to wait for it to finish. The idea is that any function, which schedules some job off the current thread, should return a `ConcurrentJob`. Jobs are stored inside the `Jobs` table, which gives a nice overview of all currently active tasks and can be used to wait for all active tasks to finish. All `ConcurrentJob`s must be stored in a `Jobs` table, it's and error to drop a job on the floor. --- Cargo.lock | 62 +++++++++++++++++++++++ Cargo.toml | 1 + src/actions/mod.rs | 14 ++++++ src/actions/post_build.rs | 5 +- src/build/mod.rs | 1 - src/concurrency.rs | 100 ++++++++++++++++++++++++++++++++++++++ src/main.rs | 3 ++ src/server/dispatch.rs | 55 +++++++-------------- src/server/message.rs | 2 +- src/server/mod.rs | 13 ++++- src/test/harness.rs | 23 ++------- src/test/lens.rs | 18 ++++--- src/test/mod.rs | 67 +++++++++++++++++++------ 13 files changed, 280 insertions(+), 84 deletions(-) create mode 100644 src/concurrency.rs diff --git a/Cargo.lock b/Cargo.lock index e0c9f458577..7c56b8f98f6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -178,6 +178,14 @@ dependencies = [ "url 1.7.1 (registry+https://github.com/rust-lang/crates.io-index)", ] +[[package]] +name = "cloudabi" +version = "0.0.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +dependencies = [ + "bitflags 1.0.3 (registry+https://github.com/rust-lang/crates.io-index)", +] + [[package]] name = "cmake" version = "0.1.31" @@ -234,6 +242,18 @@ name = "crossbeam" version = "0.3.2" source = "registry+https://github.com/rust-lang/crates.io-index" +[[package]] +name = "crossbeam-channel" +version = "0.2.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +dependencies = [ + "crossbeam-epoch 0.5.0 (registry+https://github.com/rust-lang/crates.io-index)", + "crossbeam-utils 0.4.1 (registry+https://github.com/rust-lang/crates.io-index)", + "parking_lot 0.5.5 (registry+https://github.com/rust-lang/crates.io-index)", + "rand 0.5.4 (registry+https://github.com/rust-lang/crates.io-index)", + "smallvec 0.6.2 (registry+https://github.com/rust-lang/crates.io-index)", +] + [[package]] name = "crossbeam-deque" version = "0.2.0" @@ -257,6 +277,19 @@ dependencies = [ "scopeguard 0.3.3 (registry+https://github.com/rust-lang/crates.io-index)", ] +[[package]] +name = "crossbeam-epoch" +version = "0.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +dependencies = [ + "arrayvec 0.4.7 (registry+https://github.com/rust-lang/crates.io-index)", + "cfg-if 0.1.4 (registry+https://github.com/rust-lang/crates.io-index)", + "crossbeam-utils 0.4.1 (registry+https://github.com/rust-lang/crates.io-index)", + "lazy_static 1.0.1 (registry+https://github.com/rust-lang/crates.io-index)", + "memoffset 0.2.1 (registry+https://github.com/rust-lang/crates.io-index)", + "scopeguard 0.3.3 (registry+https://github.com/rust-lang/crates.io-index)", +] + [[package]] name = "crossbeam-utils" version = "0.2.2" @@ -265,6 +298,11 @@ dependencies = [ "cfg-if 0.1.4 (registry+https://github.com/rust-lang/crates.io-index)", ] +[[package]] +name = "crossbeam-utils" +version = "0.4.1" +source = "registry+https://github.com/rust-lang/crates.io-index" + [[package]] name = "crypto-hash" version = "0.3.1" @@ -926,6 +964,23 @@ dependencies = [ "winapi 0.3.5 (registry+https://github.com/rust-lang/crates.io-index)", ] +[[package]] +name = "rand" +version = "0.5.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +dependencies = [ + "cloudabi 0.0.3 (registry+https://github.com/rust-lang/crates.io-index)", + "fuchsia-zircon 0.3.3 (registry+https://github.com/rust-lang/crates.io-index)", + "libc 0.2.42 (registry+https://github.com/rust-lang/crates.io-index)", + "rand_core 0.2.1 (registry+https://github.com/rust-lang/crates.io-index)", + "winapi 0.3.5 (registry+https://github.com/rust-lang/crates.io-index)", +] + +[[package]] +name = "rand_core" +version = "0.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" + [[package]] name = "rayon" version = "1.0.1" @@ -1015,6 +1070,7 @@ dependencies = [ "cargo 0.30.0 (git+https://github.com/rust-lang/cargo?rev=af9e40c26b4ea2ebd6f31ee86ee61d5ac1c74eb0)", "cargo_metadata 0.5.8 (registry+https://github.com/rust-lang/crates.io-index)", "clippy_lints 0.0.212 (git+https://github.com/rust-lang-nursery/rust-clippy?rev=f27aaacb9bf1d0d3492f20f92346bb1aaf45e3d8)", + "crossbeam-channel 0.2.2 (registry+https://github.com/rust-lang/crates.io-index)", "env_logger 0.5.10 (registry+https://github.com/rust-lang/crates.io-index)", "failure 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)", "itertools 0.7.8 (registry+https://github.com/rust-lang/crates.io-index)", @@ -1676,6 +1732,7 @@ dependencies = [ "checksum cfg-if 0.1.4 (registry+https://github.com/rust-lang/crates.io-index)" = "efe5c877e17a9c717a0bf3613b2709f723202c4e4675cc8f12926ded29bcb17e" "checksum clap 2.32.0 (registry+https://github.com/rust-lang/crates.io-index)" = "b957d88f4b6a63b9d70d5f454ac8011819c6efa7727858f458ab71c756ce2d3e" "checksum clippy_lints 0.0.212 (git+https://github.com/rust-lang-nursery/rust-clippy?rev=f27aaacb9bf1d0d3492f20f92346bb1aaf45e3d8)" = "" +"checksum cloudabi 0.0.3 (registry+https://github.com/rust-lang/crates.io-index)" = "ddfc5b9aa5d4507acaf872de71051dfd0e309860e88966e1051e462a077aac4f" "checksum cmake 0.1.31 (registry+https://github.com/rust-lang/crates.io-index)" = "95470235c31c726d72bf2e1f421adc1e65b9d561bf5529612cbe1a72da1467b3" "checksum commoncrypto 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)" = "d056a8586ba25a1e4d61cb090900e495952c7886786fc55f909ab2f819b69007" "checksum commoncrypto-sys 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)" = "1fed34f46747aa73dfaa578069fd8279d2818ade2b55f38f22a9401c7f4083e2" @@ -1683,9 +1740,12 @@ dependencies = [ "checksum core-foundation-sys 0.6.0 (registry+https://github.com/rust-lang/crates.io-index)" = "b2a53cce0ddcf7e7e1f998738d757d5a3bf08bf799a180e50ebe50d298f52f5a" "checksum crates-io 0.18.0 (git+https://github.com/rust-lang/cargo?rev=af9e40c26b4ea2ebd6f31ee86ee61d5ac1c74eb0)" = "" "checksum crossbeam 0.3.2 (registry+https://github.com/rust-lang/crates.io-index)" = "24ce9782d4d5c53674646a6a4c1863a21a8fc0cb649b3c94dfc16e45071dea19" +"checksum crossbeam-channel 0.2.2 (registry+https://github.com/rust-lang/crates.io-index)" = "258f3c07af0255827670241eacc8b0af7dbfc363df537ad062c6c515ca4a32ee" "checksum crossbeam-deque 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)" = "f739f8c5363aca78cfb059edf753d8f0d36908c348f3d8d1503f03d8b75d9cf3" "checksum crossbeam-epoch 0.3.1 (registry+https://github.com/rust-lang/crates.io-index)" = "927121f5407de9956180ff5e936fe3cf4324279280001cd56b669d28ee7e9150" +"checksum crossbeam-epoch 0.5.0 (registry+https://github.com/rust-lang/crates.io-index)" = "48deb8586d997ab13e98fb7e057b232149f9440321c73845b2f4cee483da29bc" "checksum crossbeam-utils 0.2.2 (registry+https://github.com/rust-lang/crates.io-index)" = "2760899e32a1d58d5abb31129f8fae5de75220bc2176e77ff7c627ae45c918d9" +"checksum crossbeam-utils 0.4.1 (registry+https://github.com/rust-lang/crates.io-index)" = "ea52fab26a99d96cdff39d0ca75c9716125937f5dba2ab83923aaaf5928f684a" "checksum crypto-hash 0.3.1 (registry+https://github.com/rust-lang/crates.io-index)" = "09de9ee0fc255ace04c7fa0763c9395a945c37c8292bb554f8d48361d1dcf1b4" "checksum curl 0.4.12 (registry+https://github.com/rust-lang/crates.io-index)" = "aaf20bbe084f285f215eef2165feed70d6b75ba29cad24469badb853a4a287d0" "checksum curl-sys 0.4.5 (registry+https://github.com/rust-lang/crates.io-index)" = "71c63a540a9ee4e15e56c3ed9b11a2f121239b9f6d7b7fe30f616e048148df9a" @@ -1764,6 +1824,8 @@ dependencies = [ "checksum quote 0.6.3 (registry+https://github.com/rust-lang/crates.io-index)" = "e44651a0dc4cdd99f71c83b561e221f714912d11af1a4dff0631f923d53af035" "checksum racer 2.0.14 (registry+https://github.com/rust-lang/crates.io-index)" = "e713729f45f12df5c5e182d39506766f76c09133fb661d3622e0ddf8078911c2" "checksum rand 0.4.2 (registry+https://github.com/rust-lang/crates.io-index)" = "eba5f8cb59cc50ed56be8880a5c7b496bfd9bd26394e176bc67884094145c2c5" +"checksum rand 0.5.4 (registry+https://github.com/rust-lang/crates.io-index)" = "12397506224b2f93e6664ffc4f664b29be8208e5157d3d90b44f09b5fae470ea" +"checksum rand_core 0.2.1 (registry+https://github.com/rust-lang/crates.io-index)" = "edecf0f94da5551fc9b492093e30b041a891657db7940ee221f9d2f66e82eef2" "checksum rayon 1.0.1 (registry+https://github.com/rust-lang/crates.io-index)" = "80e811e76f1dbf68abf87a759083d34600017fc4e10b6bd5ad84a700f9dba4b1" "checksum rayon-core 1.4.0 (registry+https://github.com/rust-lang/crates.io-index)" = "9d24ad214285a7729b174ed6d3bcfcb80177807f959d95fafd5bfc5c4f201ac8" "checksum redox_syscall 0.1.40 (registry+https://github.com/rust-lang/crates.io-index)" = "c214e91d3ecf43e9a4e41e578973adeb14b474f2bee858742d127af75a0112b1" diff --git a/Cargo.toml b/Cargo.toml index 04cb208ea76..4bdebd50d06 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -40,6 +40,7 @@ url = "1.1.0" walkdir = "2.1" regex = "1" ordslice = "0.3" +crossbeam-channel = "0.2.1" [dev-dependencies] json = "0.11" diff --git a/src/actions/mod.rs b/src/actions/mod.rs index b27ba7bee19..4cfcb4e53cd 100644 --- a/src/actions/mod.rs +++ b/src/actions/mod.rs @@ -27,6 +27,7 @@ use crate::build::*; use crate::lsp_data; use crate::lsp_data::*; use crate::server::Output; +use crate::concurrency::{ConcurrentJob, Jobs}; use std::collections::HashMap; use std::path::{Path, PathBuf}; @@ -150,6 +151,7 @@ pub struct InitActionContext { prev_changes: Arc>>, config: Arc>, + jobs: Arc>, client_capabilities: Arc, client_supports_cmd_run: bool, /// Whether the server is performing cleanup (after having received @@ -199,6 +201,7 @@ impl InitActionContext { analysis_queue, vfs, config, + jobs: Arc::new(Mutex::new(Jobs::new())), current_project, previous_build_results: Arc::new(Mutex::new(HashMap::new())), build_queue, @@ -238,6 +241,8 @@ impl InitActionContext { } fn build(&self, project_path: &Path, priority: BuildPriority, out: &O) { + let (job, token) = ConcurrentJob::new(); + self.add_job(job); let pbh = { let config = self.config.lock().unwrap(); @@ -253,6 +258,7 @@ impl InitActionContext { use_black_list: config.use_crate_blacklist, notifier: Box::new(BuildDiagnosticsNotifier::new(out.clone())), blocked_threads: vec![], + token, } }; @@ -267,6 +273,14 @@ impl InitActionContext { self.build(&self.current_project, priority, out); } + pub fn add_job(&self, job: ConcurrentJob) { + self.jobs.lock().unwrap().add(job); + } + + pub fn wait_for_concurrent_jobs(&self) { + self.jobs.lock().unwrap().wait_for_all(); + } + /// Block until any builds and analysis tasks are complete. fn block_on_build(&self) { self.build_queue.block_on_build(); diff --git a/src/actions/post_build.rs b/src/actions/post_build.rs index 20c2fb34e61..a1fd45abd50 100644 --- a/src/actions/post_build.rs +++ b/src/actions/post_build.rs @@ -24,9 +24,10 @@ use std::thread::{self, Thread}; use crate::actions::diagnostics::{parse_diagnostics, Diagnostic, ParsedDiagnostics, Suggestion}; use crate::actions::progress::DiagnosticsNotifier; use crate::build::BuildResult; +use crate::lsp_data::PublishDiagnosticsParams; +use crate::concurrency::JobToken; use languageserver_types::DiagnosticSeverity; use itertools::Itertools; -use crate::lsp_data::PublishDiagnosticsParams; use rls_analysis::AnalysisHost; use rls_data::Analysis; @@ -46,6 +47,8 @@ pub struct PostBuildHandler { pub active_build_count: Arc, pub notifier: Box, pub blocked_threads: Vec, + #[allow(unused)] // for drop + pub token: JobToken, } impl PostBuildHandler { diff --git a/src/build/mod.rs b/src/build/mod.rs index 0ca876bba37..28caf375da9 100644 --- a/src/build/mod.rs +++ b/src/build/mod.rs @@ -270,7 +270,6 @@ impl BuildQueue { if needs_compilation_ctx_from_cargo { priority = BuildPriority::Cargo; } - let build = PendingBuild { build_dir: new_build_dir.to_owned(), built_files: self.internals.dirty_files.lock().unwrap().clone(), diff --git a/src/concurrency.rs b/src/concurrency.rs new file mode 100644 index 00000000000..5197972312a --- /dev/null +++ b/src/concurrency.rs @@ -0,0 +1,100 @@ +use std::{thread}; + +use crossbeam_channel::{bounded, Receiver, Sender}; + +/// `ConcurrentJob` is a handle for some long-running computation +/// off the main thread. It can be used, indirectly, to wait for +/// the completion of the said computation. +/// +/// All `ConncurrentJob`s must eventually be stored in a `Jobs` table. +/// +/// All concurrent activities, like spawning a thread or pushing +/// a work item to a job queue, should be covered by `ConcurrentJob`. +/// This way, the set of `Jobs` table will give a complete overview of +/// concurrency in the system, and it will be possinle to wait for all +/// jobs to finish, which helps tremendously with making tests deterministic. +/// +/// `JobToken` is the worker-side counterpart of `ConcurrentJob`. Dropping +/// a `JobToken` signals that the corresponding job has finished. +#[must_use] +pub struct ConcurrentJob { + chan: Receiver, +} + +pub struct JobToken { + #[allow(unused)] // for drop + chan: Sender, +} + +pub struct Jobs { + jobs: Vec, +} + +impl Jobs { + pub fn new() -> Jobs { + Jobs { jobs: Vec::new() } + } + + pub fn add(&mut self, job: ConcurrentJob) { + self.gc(); + self.jobs.push(job); + } + + /// Blocks the current thread until all pending jobs are finished. + pub fn wait_for_all(&mut self) { + while !self.jobs.is_empty() { + let done: usize = { + let chans = self.jobs.iter().map(|j| &j.chan); + select! { + recv(chans, msg, from) => { + assert!(msg.is_none()); + self.jobs.iter().position(|j| &j.chan == from).unwrap() + } + } + }; + drop(self.jobs.swap_remove(done)); + } + } + + fn gc(&mut self) { + self.jobs.retain(|job| !job.is_completed()) + } +} + +impl ConcurrentJob { + pub fn new() -> (ConcurrentJob, JobToken) { + let (tx, rx) = bounded(0); + let job = ConcurrentJob { chan: rx }; + let token = JobToken { chan: tx }; + (job, token) + } + + fn is_completed(&self) -> bool { + is_closed(&self.chan) + } +} + +impl Drop for ConcurrentJob { + fn drop(&mut self) { + if self.is_completed() || thread::panicking() { + return; + } + panic!("orphaned concurrent job"); + } +} + +// We don't actually send messages through the channels, +// and instead just check if the channel is closed, +// so we use uninhabited enum as a message type +enum Never {} + +/// Nonblocking +fn is_closed(chan: &Receiver) -> bool { + select! { + recv(chan, msg) => match msg { + None => true, + Some(never) => match never {} + } + default => false, + } +} diff --git a/src/main.rs b/src/main.rs index a891872c4a6..f8b3d09a32d 100644 --- a/src/main.rs +++ b/src/main.rs @@ -43,6 +43,8 @@ extern crate log; extern crate serde_derive; #[macro_use] extern crate serde_json; +#[macro_use] +extern crate crossbeam_channel; use std::env; use std::sync::Arc; @@ -57,6 +59,7 @@ pub mod cmd; pub mod config; pub mod lsp_data; pub mod server; +pub mod concurrency; #[cfg(test)] mod test; diff --git a/src/server/dispatch.rs b/src/server/dispatch.rs index 7c38727183d..3595163214b 100644 --- a/src/server/dispatch.rs +++ b/src/server/dispatch.rs @@ -18,6 +18,7 @@ use crate::server; use crate::server::io::Output; use crate::server::message::ResponseError; use crate::server::{Request, Response}; +use crate::concurrency::{ConcurrentJob, JobToken}; use std::sync::mpsc; use std::thread; use std::time::Duration; @@ -37,23 +38,23 @@ macro_rules! define_dispatch_request_enum { #[allow(large_enum_variant)] // seems ok for a short lived macro-enum crate enum DispatchRequest { $( - $request_type(Request<$request_type>, InitActionContext), + $request_type(Request<$request_type>), )* } $( - impl From<(Request<$request_type>, InitActionContext)> for DispatchRequest { - fn from((req, ctx): (Request<$request_type>, InitActionContext)) -> Self { - DispatchRequest::$request_type(req, ctx) + impl From> for DispatchRequest { + fn from(req: Request<$request_type>) -> Self { + DispatchRequest::$request_type(req) } } )* impl DispatchRequest { - fn handle(self, out: &O) { + fn handle(self, ctx: InitActionContext, out: &O) { match self { $( - DispatchRequest::$request_type(req, ctx) => { + DispatchRequest::$request_type(req) => { let Request { id, params, received, .. } = req; let timeout = $request_type::timeout(); @@ -111,55 +112,33 @@ define_dispatch_request_enum!( /// Requests dispatched this way are automatically timed out & avoid /// processing if have already timed out before starting. crate struct Dispatcher { - sender: mpsc::Sender, - - request_handled_receiver: mpsc::Receiver<()>, - /// Number of as-yet-unhandled requests dispatched to the worker thread - in_flight_requests: usize, + sender: mpsc::Sender<(DispatchRequest, InitActionContext, JobToken)>, } impl Dispatcher { /// Creates a new `Dispatcher` starting a new thread and channel crate fn new(out: O) -> Self { - let (sender, receiver) = mpsc::channel::(); - let (request_handled_sender, request_handled_receiver) = mpsc::channel::<()>(); + let (sender, receiver) = mpsc::channel::<(DispatchRequest, InitActionContext, JobToken)>(); thread::Builder::new() .name("dispatch-worker".into()) .spawn(move || { - while let Ok(request) = receiver.recv() { - request.handle(&out); - let _ = request_handled_sender.send(()); + while let Ok((request, ctx, token)) = receiver.recv() { + request.handle(ctx, &out); + drop(token); } }) .unwrap(); - Self { - sender, - request_handled_receiver, - in_flight_requests: 0, - } - } - - /// Blocks until all dispatched requests have been handled - crate fn await_all_dispatched(&mut self) { - while self.in_flight_requests != 0 { - self.request_handled_receiver.recv().unwrap(); - self.in_flight_requests -= 1; - } + Self { sender } } /// Sends a request to the dispatch-worker thread, does not block - crate fn dispatch>(&mut self, request: R) { - if let Err(err) = self.sender.send(request.into()) { + crate fn dispatch>(&mut self, request: R, ctx: InitActionContext) { + let (job, token) = ConcurrentJob::new(); + ctx.add_job(job); + if let Err(err) = self.sender.send((request.into(), ctx, token)) { debug!("Failed to dispatch request: {:?}", err); - } else { - self.in_flight_requests += 1; - } - - // Clear the handled queue if possible in a non-blocking way - while self.request_handled_receiver.try_recv().is_ok() { - self.in_flight_requests -= 1; } } } diff --git a/src/server/message.rs b/src/server/message.rs index f2d99bc6c2e..4ddd4d25647 100644 --- a/src/server/message.rs +++ b/src/server/message.rs @@ -68,7 +68,7 @@ impl From<()> for ResponseError { /// Blocks stdin whilst being handled. pub trait BlockingNotificationAction: LSPNotification { /// Handle this notification. - fn handle(_: Self::Params, _: &mut InitActionContext, _: O) -> Result<(), ()>; + fn handle(_:Self::Params,_: &mut InitActionContext,_: O,) -> Result<(), ()>; } /// A request that blocks stdin whilst being handled diff --git a/src/server/mod.rs b/src/server/mod.rs index a067c08ab82..4f7f5cef644 100644 --- a/src/server/mod.rs +++ b/src/server/mod.rs @@ -195,7 +195,7 @@ impl LsService { let request: Request<$br_action> = msg.parse_as_request()?; // block until all nonblocking requests have been handled ensuring ordering - self.dispatcher.await_all_dispatched(); + self.wait_for_concurrent_jobs(); let req_id = request.id.clone(); match request.blocking_dispatch(&mut self.ctx, &self.output) { @@ -220,7 +220,7 @@ impl LsService { <$request as LSPRequest>::METHOD => { let request: Request<$request> = msg.parse_as_request()?; if let Ok(ctx) = self.ctx.inited() { - self.dispatcher.dispatch((request, ctx)); + self.dispatcher.dispatch(request, ctx); } else { warn!( @@ -335,6 +335,15 @@ impl LsService { ServerStateChange::Continue } + + pub fn wait_for_concurrent_jobs(&mut self) { + match &self.ctx { + ActionContext::Init(ctx) => { + ctx.wait_for_concurrent_jobs() + } + ActionContext::Uninit(_) => {} + } + } } /// How should the server proceed? diff --git a/src/test/harness.rs b/src/test/harness.rs index 131d62a2516..1a02eb9533d 100644 --- a/src/test/harness.rs +++ b/src/test/harness.rs @@ -196,26 +196,13 @@ impl ExpectedMessage { } } -macro_rules! wait_for_n_results { - ($n:expr, $results:expr) => {{ - use std::time::{Duration, SystemTime}; - use std::thread; - - let timeout = Duration::from_secs(320); - let start_clock = SystemTime::now(); - let mut results_count = $results.lock().unwrap().len(); - while results_count < $n { - if start_clock.elapsed().unwrap() >= timeout { - panic!("Timeout waiting for a result"); - } - thread::sleep(Duration::from_millis(100)); - results_count = $results.lock().unwrap().len(); - } - }}; +crate fn clear_messages(server: &mut ls_server::LsService, results: LsResultList) { + server.wait_for_concurrent_jobs(); + results.lock().unwrap().clear(); } -crate fn expect_messages(results: LsResultList, expected: &[&ExpectedMessage]) { - wait_for_n_results!(expected.len(), results); +crate fn expect_messages(server: &mut ls_server::LsService, results: LsResultList, expected: &[&ExpectedMessage]) { + server.wait_for_concurrent_jobs(); let mut results = results.lock().unwrap(); diff --git a/src/test/lens.rs b/src/test/lens.rs index bfa080d832a..96556d2d0fd 100644 --- a/src/test/lens.rs +++ b/src/test/lens.rs @@ -4,19 +4,20 @@ use std::{ use url::Url; use serde_json; -use ls_types::{ - TextDocumentIdentifier, CodeLensParams -}; +use languageserver_types::{TextDocumentIdentifier, CodeLensParams}; -use ::{ +use crate::{ server as ls_server, actions::requests, -}; -use super::{ - Environment, expect_messages, request, ExpectedMessage, initialize_with_opts, InitializationOptions + lsp_data::InitializationOptions, + test::{ + request, initialize_with_opts, + harness::{expect_messages, Environment, ExpectedMessage}, + }, }; #[test] +#[ignore] // FIXME(#925) intermittent failure fn test_lens_run() { let mut env = Environment::new("lens_run"); @@ -47,6 +48,7 @@ fn test_lens_run() { ls_server::ServerStateChange::Continue ); expect_messages( + &mut server, results.clone(), &[ ExpectedMessage::new(Some(0)).expect_contains(r#""codeLensProvider":{"resolveProvider":false}"#), @@ -63,7 +65,7 @@ fn test_lens_run() { ls_server::LsService::handle_message(&mut server), ls_server::ServerStateChange::Continue ); - wait_for_n_results!(1, results); + server.wait_for_concurrent_jobs(); let result: serde_json::Value = serde_json::from_str(&results.lock().unwrap().remove(0)).unwrap(); compare_json( result.get("result").unwrap(), diff --git a/src/test/mod.rs b/src/test/mod.rs index 7e92667834f..531d94ca6a9 100644 --- a/src/test/mod.rs +++ b/src/test/mod.rs @@ -15,8 +15,7 @@ use json; #[macro_use] mod harness; -// FIXME(#925) intermittent failure -//mod lens; +mod lens; use rls_analysis::{AnalysisHost, Target}; use crate::actions::{requests, notifications}; @@ -117,6 +116,7 @@ fn test_shutdown() { ls_server::ServerStateChange::Continue ); expect_messages( + &mut server, results.clone(), &[ ExpectedMessage::new(Some(0)).expect_contains("capabilities"), @@ -133,7 +133,7 @@ fn test_shutdown() { ls_server::LsService::handle_message(&mut server), ls_server::ServerStateChange::Continue ); - expect_messages(results.clone(), &[&ExpectedMessage::new(Some(1))]); + expect_messages(&mut server, results.clone(), &[&ExpectedMessage::new(Some(1))]); } #[test] @@ -165,6 +165,7 @@ fn test_goto_def() { ls_server::ServerStateChange::Continue ); expect_messages( + &mut server, results.clone(), &[ ExpectedMessage::new(Some(0)).expect_contains("capabilities"), @@ -183,6 +184,7 @@ fn test_goto_def() { ); // TODO structural checking of result, rather than looking for a string - src(&source_file_path, 12, "world") expect_messages( + &mut server, results.clone(), &[ ExpectedMessage::new(Some(11)).expect_contains(r#""start":{"line":20,"character":8}"#), @@ -219,6 +221,7 @@ fn test_hover() { ls_server::ServerStateChange::Continue ); expect_messages( + &mut server, results.clone(), &[ ExpectedMessage::new(Some(0)).expect_contains("capabilities"), @@ -236,6 +239,7 @@ fn test_hover() { ls_server::ServerStateChange::Continue ); expect_messages( + &mut server, results.clone(), &[ ExpectedMessage::new(Some(11)) @@ -305,6 +309,7 @@ fn test_hover_after_src_line_change() { ls_server::ServerStateChange::Continue ); expect_messages( + &mut server, results.clone(), &[ ExpectedMessage::new(Some(0)).expect_contains("capabilities"), @@ -323,6 +328,7 @@ fn test_hover_after_src_line_change() { ls_server::ServerStateChange::Continue ); expect_messages( + &mut server, results.clone(), &[ ExpectedMessage::new(Some(11)) @@ -337,6 +343,7 @@ fn test_hover_after_src_line_change() { ); expect_messages( + &mut server, results.clone(), &[ ExpectedMessage::new(None).expect_contains("progress").expect_contains(r#"title":"Building""#), @@ -354,6 +361,7 @@ fn test_hover_after_src_line_change() { ls_server::ServerStateChange::Continue ); expect_messages( + &mut server, results.clone(), &[ ExpectedMessage::new(None) @@ -386,6 +394,7 @@ fn test_workspace_symbol() { ls_server::ServerStateChange::Continue ); expect_messages( + &mut server, results.clone(), &[ ExpectedMessage::new(Some(0)).expect_contains("capabilities"), @@ -402,7 +411,8 @@ fn test_workspace_symbol() { ls_server::ServerStateChange::Continue ); - expect_messages(results.clone(), &[ExpectedMessage::new(Some(42)).expect_contains(r#""id":42"#) + expect_messages( + &mut server,results.clone(), &[ExpectedMessage::new(Some(42)).expect_contains(r#""id":42"#) // in main.rs .expect_contains(r#"main.rs"#) .expect_contains(r#""name":"nemo""#) @@ -450,6 +460,7 @@ fn test_find_all_refs() { ls_server::ServerStateChange::Continue ); expect_messages( + &mut server, results.clone(), &[ ExpectedMessage::new(Some(0)).expect_contains("capabilities"), @@ -466,6 +477,7 @@ fn test_find_all_refs() { ls_server::ServerStateChange::Continue ); expect_messages( + &mut server, results.clone(), &[ ExpectedMessage::new(Some(42)) @@ -514,6 +526,7 @@ fn test_find_all_refs_no_cfg_test() { ls_server::ServerStateChange::Continue ); expect_messages( + &mut server, results.clone(), &[ ExpectedMessage::new(Some(0)).expect_contains("capabilities"), @@ -530,6 +543,7 @@ fn test_find_all_refs_no_cfg_test() { ls_server::ServerStateChange::Continue ); expect_messages( + &mut server, results.clone(), &[ ExpectedMessage::new(Some(42)) @@ -559,6 +573,7 @@ fn test_borrow_error() { ls_server::ServerStateChange::Continue ); expect_messages( + &mut server, results.clone(), &[ ExpectedMessage::new(Some(0)).expect_contains("capabilities"), @@ -604,6 +619,7 @@ fn test_highlight() { ls_server::ServerStateChange::Continue ); expect_messages( + &mut server, results.clone(), &[ ExpectedMessage::new(Some(0)).expect_contains("capabilities"), @@ -621,6 +637,7 @@ fn test_highlight() { ls_server::ServerStateChange::Continue ); expect_messages( + &mut server, results.clone(), &[ ExpectedMessage::new(Some(42)) @@ -664,6 +681,7 @@ fn test_rename() { ls_server::ServerStateChange::Continue ); expect_messages( + &mut server, results.clone(), &[ ExpectedMessage::new(Some(0)).expect_contains("capabilities"), @@ -681,6 +699,7 @@ fn test_rename() { ls_server::ServerStateChange::Continue ); expect_messages( + &mut server, results.clone(), &[ ExpectedMessage::new(Some(42)) @@ -727,6 +746,7 @@ fn test_reformat() { ls_server::ServerStateChange::Continue ); expect_messages( + &mut server, results.clone(), &[ ExpectedMessage::new(Some(0)).expect_contains("capabilities"), @@ -743,7 +763,8 @@ fn test_reformat() { ls_server::LsService::handle_message(&mut server), ls_server::ServerStateChange::Continue ); - expect_messages(results.clone(), &[ExpectedMessage::new(Some(42)).expect_contains(r#"{"start":{"line":0,"character":0},"end":{"line":12,"character":0}}"#) + expect_messages( + &mut server,results.clone(), &[ExpectedMessage::new(Some(42)).expect_contains(r#"{"start":{"line":0,"character":0},"end":{"line":12,"character":0}}"#) .expect_contains(r#"newText":"// Copyright 2017 The Rust Project Developers. See the COPYRIGHT\n// file at the top-level directory of this distribution and at\n// http://rust-lang.org/COPYRIGHT.\n//\n// Licensed under the Apache License, Version 2.0 or the MIT license\n// , at your\n// option. This file may not be copied, modified, or distributed\n// except according to those terms.\n\npub mod foo;\npub fn main() {\n let world = \"world\";\n println!(\"Hello, {}!\", world);\n}"#)]); } @@ -789,6 +810,7 @@ fn test_reformat_with_range() { ls_server::ServerStateChange::Continue ); expect_messages( + &mut server, results.clone(), &[ ExpectedMessage::new(Some(0)).expect_contains("capabilities"), @@ -805,7 +827,7 @@ fn test_reformat_with_range() { ls_server::LsService::handle_message(&mut server), ls_server::ServerStateChange::Continue ); - expect_messages(results.clone(), &[ExpectedMessage::new(Some(42)).expect_contains(r#"{"start":{"line":0,"character":0},"end":{"line":15,"character":5}}"#) + expect_messages(&mut server, results.clone(), &[ExpectedMessage::new(Some(42)).expect_contains(r#"{"start":{"line":0,"character":0},"end":{"line":15,"character":5}}"#) .expect_contains(r#"newText":"// Copyright 2017 The Rust Project Developers. See the COPYRIGHT\n// file at the top-level directory of this distribution and at\n// http://rust-lang.org/COPYRIGHT.\n//\n// Licensed under the Apache License, Version 2.0 or the MIT license\n// , at your\n// option. This file may not be copied, modified, or distributed\n// except according to those terms.\n\npub fn main() {\n let world1 = \"world\";\n println!(\"Hello, {}!\", world1);\n let world2 = \"world\";\n println!(\"Hello, {}!\", world2);\n let world3 = \"world\";\n println!(\"Hello, {}!\", world3);\n}\n"#)]); } @@ -828,6 +850,7 @@ fn test_multiple_binaries() { ls_server::ServerStateChange::Continue ); expect_messages( + &mut server, results.clone(), &[ ExpectedMessage::new(Some(0)).expect_contains("capabilities"), @@ -909,6 +932,7 @@ fn test_bin_lib_project() { ls_server::ServerStateChange::Continue ); expect_messages( + &mut server, results.clone(), &[ ExpectedMessage::new(Some(0)).expect_contains("capabilities"), @@ -993,6 +1017,7 @@ fn test_infer_lib() { ls_server::ServerStateChange::Continue ); expect_messages( + &mut server, results.clone(), &[ ExpectedMessage::new(Some(0)).expect_contains("capabilities"), @@ -1023,6 +1048,7 @@ fn test_infer_bin() { ls_server::ServerStateChange::Continue ); expect_messages( + &mut server, results.clone(), &[ ExpectedMessage::new(Some(0)).expect_contains("capabilities"), @@ -1053,6 +1079,7 @@ fn test_infer_custom_bin() { ls_server::ServerStateChange::Continue ); expect_messages( + &mut server, results.clone(), &[ ExpectedMessage::new(Some(0)).expect_contains("capabilities"), @@ -1087,7 +1114,7 @@ fn test_omit_init_build() { ls_server::LsService::handle_message(&mut server), ls_server::ServerStateChange::Continue ); - expect_messages( + expect_messages(&mut server, results.clone(), &[ ExpectedMessage::new(Some(0)).expect_contains("capabilities"), @@ -1179,7 +1206,7 @@ fn test_find_impls() { ls_server::LsService::handle_message(&mut server), ls_server::ServerStateChange::Continue ); - expect_messages( + expect_messages(&mut server, results.clone(), &[ ExpectedMessage::new(Some(0)).expect_contains("capabilities"), @@ -1197,7 +1224,7 @@ fn test_find_impls() { ls_server::ServerStateChange::Continue ); // TODO structural checking of result, rather than looking for a string - src(&source_file_path, 12, "world") - expect_messages(results.clone(), &[ + expect_messages(&mut server, results.clone(), &[ ExpectedMessage::new(Some(1)) .expect_contains(r#""range":{"start":{"line":18,"character":15},"end":{"line":18,"character":18}}"#) .expect_contains(r#""range":{"start":{"line":19,"character":12},"end":{"line":19,"character":15}}"#) @@ -1206,7 +1233,7 @@ fn test_find_impls() { ls_server::LsService::handle_message(&mut server), ls_server::ServerStateChange::Continue ); - expect_messages(results.clone(), &[ + expect_messages(&mut server, results.clone(), &[ ExpectedMessage::new(Some(2)) .expect_contains(r#""range":{"start":{"line":18,"character":15},"end":{"line":18,"character":18}}"#) .expect_contains(r#""range":{"start":{"line":22,"character":15},"end":{"line":22,"character":18}}"#) @@ -1238,6 +1265,7 @@ fn test_features() { ls_server::ServerStateChange::Continue ); expect_messages( + &mut server, results.clone(), &[ ExpectedMessage::new(Some(0)).expect_contains("capabilities"), @@ -1271,6 +1299,7 @@ fn test_all_features() { ls_server::ServerStateChange::Continue ); expect_messages( + &mut server, results.clone(), &[ ExpectedMessage::new(Some(0)).expect_contains("capabilities"), @@ -1304,6 +1333,7 @@ fn test_no_default_features() { ls_server::ServerStateChange::Continue ); expect_messages( + &mut server, results.clone(), &[ ExpectedMessage::new(Some(0)).expect_contains("capabilities"), @@ -1424,6 +1454,7 @@ fn test_deglob() { ls_server::ServerStateChange::Continue ); expect_messages( + &mut server, results.clone(), &[ ExpectedMessage::new(Some(0)).expect_contains("rls.deglobImports-"), @@ -1441,7 +1472,7 @@ fn test_deglob() { ls_server::ServerStateChange::Continue ); { - wait_for_n_results!(1, results); + server.wait_for_concurrent_jobs(); let response = json::parse(&results.lock().unwrap().remove(0)).unwrap(); assert_eq!(response["id"], 100); assert_eq!(response["result"][0]["title"], "Deglob import"); @@ -1474,7 +1505,7 @@ fn test_deglob() { ls_server::ServerStateChange::Continue ); { - wait_for_n_results!(2, results); + server.wait_for_concurrent_jobs(); let response = json::parse(&results.lock().unwrap().remove(0)).unwrap(); assert_eq!(response["id"], 0x0100_0001); assert_eq!(response["method"], "workspace/applyEdit"); @@ -1505,6 +1536,7 @@ fn test_deglob() { ls_server::ServerStateChange::Continue ); expect_messages( + &mut server, results.clone(), &[ ExpectedMessage::new(Some(1100)) @@ -1525,7 +1557,7 @@ fn test_deglob() { ); { - wait_for_n_results!(1, results); + server.wait_for_concurrent_jobs(); let response = json::parse(&results.lock().unwrap().remove(0)).unwrap(); assert_eq!(response["id"], 0x0100_0002); assert_eq!(response["method"], "workspace/applyEdit"); @@ -1546,6 +1578,7 @@ fn test_deglob() { } expect_messages( + &mut server, results.clone(), &[ ExpectedMessage::new(Some(1200)).expect_contains(r#"null"#), @@ -1574,6 +1607,7 @@ fn test_all_targets() { ls_server::ServerStateChange::Continue ); expect_messages( + &mut server, results.clone(), &[ ExpectedMessage::new(Some(0)).expect_contains("capabilities"), @@ -1631,7 +1665,8 @@ fn ignore_uninitialized_notification() { ls_server::LsService::handle_message(&mut server), ls_server::ServerStateChange::Continue ); - expect_messages(results.clone(), &[]); + expect_messages( + &mut server,results.clone(), &[]); // Initialize and build assert_eq!( @@ -1639,6 +1674,7 @@ fn ignore_uninitialized_notification() { ls_server::ServerStateChange::Continue ); expect_messages( + &mut server, results.clone(), &[ ExpectedMessage::new(Some(1)).expect_contains("capabilities"), @@ -1683,7 +1719,7 @@ fn fail_uninitialized_request() { ls_server::ServerStateChange::Continue ); { - wait_for_n_results!(1, results); + server.wait_for_concurrent_jobs(); let response = json::parse(&results.lock().unwrap().remove(0)).unwrap(); assert_eq!(response["id"], 0); assert_eq!(response["error"]["code"], -32002); @@ -1701,6 +1737,7 @@ fn fail_uninitialized_request() { ls_server::ServerStateChange::Continue ); expect_messages( + &mut server, results.clone(), &[ ExpectedMessage::new(Some(1)).expect_contains("capabilities"),