Make erlang service scoped per project and auto-started

Summary:
D55125890 makde erlang service a global singleton. This leads to issues since each project has different code paths that now start clashing.

This separates them again and makes the services lazily-started on demand, when needed, which avoids the need for explicit initialization.

Furthermore this allows removing workarounds for test flakiness caused by the global erlang service, see D55063978.

Reviewed By: robertoaloi, TD5

Differential Revision: D58285974

fbshipit-source-id: 513ab01417c401e44fb1c691d99ac70a46e79f94

crates/elp/src/bin/glean.rs

@@ -2404,10 +2404,6 @@ mod tests {
     ) {
         let (db, files, diag) = RootDatabase::with_many_files(spec);
         let project_id = ProjectId(0);
-        if diag.use_erlang_service {
-            db.ensure_erlang_service(project_id)
-                .expect("erlang service started");
-        }
         let host = AnalysisHost::new(db);
         let glean = GleanIndexer {
             project_id,

crates/elp/src/build/load.rs

@@ -183,11 +183,6 @@ fn load_database(
 
     project_apps.app_structure().apply(db);
 
-    let project_id = ProjectId(0);
-    db.ensure_erlang_service(project_id)?;
-    if let Some(otp_project_id) = project_apps.otp_project_id {
-        db.ensure_erlang_service(otp_project_id)?;
-    }
     let changes = vfs.take_changes();
     for file in changes {
         if file.exists() {

crates/elp/src/server.rs

@@ -1132,14 +1132,6 @@ impl Server {
         let folders = ProjectFolders::new(&project_apps);
         project_apps.app_structure().apply(raw_db);
 
-        for (project_id, _) in projects.iter().enumerate() {
-            let project_id = ProjectId(project_id as u32);
-            raw_db.ensure_erlang_service(project_id)?;
-        }
-        if let Some(otp_project_id) = project_apps.otp_project_id {
-            raw_db.ensure_erlang_service(otp_project_id)?;
-        }
-
         self.file_set_config = folders.file_set_config;
 
         let register_options = lsp_types::DidChangeWatchedFilesRegistrationOptions {

crates/erlang_service/src/lib.rs

@@ -81,11 +81,6 @@ pub struct Connection {
     _for_drop: Arc<SharedState>,
 }
 
-/// Until such time as we have a fully re-entrant erlang service,
-/// we must guard access to it during tests to prevent race
-/// conditions leading to flaky tests. T182801661
-pub static ERLANG_SERVICE_GLOBAL_LOCK: Mutex<()> = Mutex::new(());
-
 #[derive(Debug, Clone, Eq, PartialEq, Hash)]
 pub enum CompileOption {
     Includes(Vec<PathBuf>),

crates/ide/src/annotations.rs

@@ -82,7 +82,7 @@ mod tests {
     #[track_caller]
     fn check(fixture: &str) {
         let trimmed_fixture = trim_indent(fixture);
-        let (analysis, pos, _diagnostics_enabled, _guard, mut annotations) =
+        let (analysis, pos, _diagnostics_enabled, mut annotations) =
             fixture::annotations(trimmed_fixture.as_str());
         let mut actual = Vec::new();
         for annotation in analysis.annotations(pos.file_id).unwrap() {

crates/ide/src/codemod_helpers.rs

@@ -758,8 +758,7 @@ mod tests {
 
     #[track_caller]
     fn check_type(fixture: &str) {
-        let (db, position, _diagnostics_enabled, _guard, expected) =
-            fixture::db_annotations(fixture);
+        let (db, position, _diagnostics_enabled, expected) = fixture::db_annotations(fixture);
         let host = AnalysisHost { db };
         let sema = Semantic::new(&host.db);
         if expected.len() != 1 {

crates/ide/src/diagnostics/misspelled_attribute.rs

@@ -177,7 +177,7 @@ mod tests {
 
     #[test]
     fn test_can_ignore_valid_spelling() {
-        let (analysis, position, _, _, _) = fixture::annotations(
+        let (analysis, position, _, _) = fixture::annotations(
             r#"
     -module(main).
     -di~alyzer({nowarn_function, f/0}).
@@ -199,7 +199,7 @@ mod tests {
 
     #[test]
     fn test_does_not_consider_the_names_of_records() {
-        let (analysis, position, _, _, _) = fixture::annotations(
+        let (analysis, position, _, _) = fixture::annotations(
             r#"
     -module(main).
     -re~cord(dyalizer, {field = "foo"}).

crates/ide/src/document_symbols.rs

@@ -226,8 +226,7 @@ mod tests {
     use crate::fixture;
 
     fn check(fixture: &str) {
-        let (analysis, pos, _diagnostics_enabled, _guard, mut expected) =
-            fixture::annotations(fixture);
+        let (analysis, pos, _diagnostics_enabled, mut expected) = fixture::annotations(fixture);
         let file_id = pos.file_id;
         let symbols = analysis.document_symbols(file_id).unwrap();
 

crates/ide/src/fixture.rs

@@ -9,16 +9,13 @@
 
 //! Utilities for creating `Analysis` instances for tests.
 
-use elp_erlang_service::ERLANG_SERVICE_GLOBAL_LOCK;
 use elp_ide_db::elp_base_db::fixture::WithFixture;
 use elp_ide_db::elp_base_db::fixture::CURSOR_MARKER;
 use elp_ide_db::elp_base_db::FileId;
 use elp_ide_db::elp_base_db::FileRange;
-use elp_ide_db::elp_base_db::ProjectId;
 use elp_ide_db::elp_base_db::SourceDatabase;
 use elp_ide_db::RootDatabase;
 use elp_project_model::test_fixture::DiagnosticsEnabled;
-use parking_lot::MutexGuard;
 
 use crate::diagnostics::DiagnosticsConfig;
 use crate::diagnostics::RemoveElpReported;
@@ -37,35 +34,13 @@ pub(crate) fn single_file(fixture: &str) -> (Analysis, FileId) {
 /// Creates analysis from a multi-file fixture, returns position marked with the [`CURSOR_MARKER`]
 pub(crate) fn position(fixture: &str) -> (Analysis, FilePosition, DiagnosticsEnabled) {
     let (db, position, diagnostics_enabled) = RootDatabase::with_position(fixture);
-    start_erlang_service_if_needed(&db, position.file_id, &diagnostics_enabled);
     let host = AnalysisHost { db };
     (host.analysis(), position, diagnostics_enabled)
 }
 
-pub(crate) fn start_erlang_service_if_needed(
-    db: &RootDatabase,
-    file_id: FileId,
-    diagnostics_enabled: &DiagnosticsEnabled,
-) -> bool {
-    if diagnostics_enabled.needs_erlang_service() {
-        // This is test driver code, so unwrap() is ok, it is a cheap
-        // way to let the dev know there is a problem.
-        // Erlang: let it crash
-        let project_id: ProjectId = db
-            .app_data(db.file_source_root(file_id))
-            .unwrap()
-            .project_id;
-        db.ensure_erlang_service(project_id).unwrap();
-        true
-    } else {
-        false
-    }
-}
-
 /// Creates analysis from a multi-file fixture
 pub(crate) fn multi_file(fixture: &str) -> Analysis {
-    let (db, fixture) = RootDatabase::with_fixture(fixture);
-    start_erlang_service_if_needed(&db, fixture.file_id(), &fixture.diagnostics_enabled);
+    let (db, _fixture) = RootDatabase::with_fixture(fixture);
     let host = AnalysisHost { db };
     host.analysis()
 }
@@ -78,12 +53,11 @@ pub fn annotations(
     Analysis,
     FilePosition,
     DiagnosticsEnabled,
-    Option<MutexGuard<()>>,
     Vec<(FileRange, String)>,
 ) {
-    let (db, position, diagnostics_enabled, guard, annotations) = db_annotations(fixture);
+    let (db, position, diagnostics_enabled, annotations) = db_annotations(fixture);
     let analysis = AnalysisHost { db }.analysis();
-    (analysis, position, diagnostics_enabled, guard, annotations)
+    (analysis, position, diagnostics_enabled, annotations)
 }
 
 /// Creates db from a multi-file fixture, returns first position marked with [`CURSOR_MARKER`]
@@ -94,17 +68,9 @@ pub fn db_annotations(
     RootDatabase,
     FilePosition,
     DiagnosticsEnabled,
-    Option<MutexGuard<()>>,
     Vec<(FileRange, String)>,
 ) {
     let (db, fixture) = RootDatabase::with_fixture(fixture);
-    let guard =
-        if start_erlang_service_if_needed(&db, fixture.file_id(), &fixture.diagnostics_enabled) {
-            Some(ERLANG_SERVICE_GLOBAL_LOCK.lock())
-        } else {
-            None
-        };
-
     let (file_id, range_or_offset) = fixture
         .file_position
         .expect(&format!("expected a marker ({})", CURSOR_MARKER));
@@ -115,7 +81,6 @@ pub fn db_annotations(
         db,
         FilePosition { file_id, offset },
         fixture.diagnostics_enabled,
-        guard,
         annotations,
     )
 }

crates/ide/src/handlers/goto_definition.rs

@@ -48,8 +48,7 @@ mod tests {
 
     #[track_caller]
     fn check_worker(fixture: &str, check_parse_error: bool) {
-        let (analysis, position, _diagnostics_enabled, _guard, expected) =
-            fixture::annotations(fixture);
+        let (analysis, position, _diagnostics_enabled, expected) = fixture::annotations(fixture);
         if check_parse_error {
             check_no_parse_errors(&analysis, position.file_id);
         }

crates/ide/src/handlers/goto_type_definition.rs

@@ -63,10 +63,7 @@ mod tests {
 
     #[track_caller]
     fn check_worker(fixture: &str, check_parse_error: bool) {
-        let (analysis, position, _diagnostics_enabled, _guard, expected) =
-            fixture::annotations(fixture);
-        let project_id = analysis.project_id(position.file_id).unwrap().unwrap();
-        let _ = analysis.db.ensure_erlang_service(project_id);
+        let (analysis, position, _diagnostics_enabled, expected) = fixture::annotations(fixture);
         if check_parse_error {
             check_no_parse_errors(&analysis, position.file_id);
         }

crates/ide/src/handlers/references.rs

@@ -96,8 +96,7 @@ mod tests {
     use crate::tests::check_file_ranges;
 
     fn check(fixture: &str) {
-        let (analysis, pos, _diagnostics_enabled, _guard, mut annos) =
-            fixture::annotations(fixture);
+        let (analysis, pos, _diagnostics_enabled, mut annos) = fixture::annotations(fixture);
         if let Ok(Some(resolved)) = analysis.find_all_refs(pos) {
             for res in resolved {
                 let def_name = match annos

crates/ide/src/highlight_related.rs

@@ -131,8 +131,7 @@ mod tests {
 
     #[track_caller]
     fn check(fixture_str: &str) {
-        let (analysis, pos, _diagnostics_enabled, _guard, annotations) =
-            fixture::annotations(fixture_str);
+        let (analysis, pos, _diagnostics_enabled, annotations) = fixture::annotations(fixture_str);
         fixture::check_no_parse_errors(&analysis, pos.file_id);
 
         let hls = analysis.highlight_related(pos).unwrap().unwrap_or_default();

crates/ide/src/runnables.rs

@@ -181,10 +181,8 @@ mod tests {
     #[track_caller]
     fn check_runnables(fixture: &str) {
         let trimmed_fixture = trim_indent(fixture);
-        let (analysis, pos, _diagnostics_enabled, _guard, mut annotations) =
+        let (analysis, pos, _diagnostics_enabled, mut annotations) =
             fixture::annotations(&trimmed_fixture.as_str());
-        let project_id = analysis.project_id(pos.file_id).unwrap().unwrap();
-        let _ = analysis.db.ensure_erlang_service(project_id);
         let runnables = analysis.runnables(pos.file_id).unwrap();
         let mut actual = Vec::new();
         for runnable in runnables {

crates/ide/src/syntax_highlighting.rs

@@ -258,7 +258,6 @@ mod tests {
     use elp_ide_db::RootDatabase;
     use itertools::Itertools;
 
-    use crate::fixture::start_erlang_service_if_needed;
     use crate::syntax_highlighting::highlight;
     use crate::HlTag;
 
@@ -275,7 +274,6 @@ mod tests {
         };
 
         let (db, fixture) = RootDatabase::with_fixture(&fixture);
-        start_erlang_service_if_needed(&db, fixture.file_id(), &fixture.diagnostics_enabled);
         let annotations = fixture.annotations(&db);
         let expected: Vec<_> = annotations
             .into_iter()

crates/ide/src/tests.rs

@@ -17,7 +17,6 @@ use elp_ide_db::elp_base_db::fixture::remove_annotations;
 use elp_ide_db::elp_base_db::fixture::WithFixture;
 use elp_ide_db::elp_base_db::FileId;
 use elp_ide_db::elp_base_db::FileRange;
-use elp_ide_db::elp_base_db::SourceDatabase;
 use elp_ide_db::elp_base_db::SourceDatabaseExt;
 use elp_ide_db::RootDatabase;
 use elp_project_model::test_fixture::trim_indent;
@@ -350,11 +349,6 @@ pub(crate) fn check_ct_diagnostics(elp_fixture: &str) {
 #[track_caller]
 pub(crate) fn check_diagnostics_with_config(config: DiagnosticsConfig, elp_fixture: &str) {
     let (db, files, diagnostics_enabled) = RootDatabase::with_many_files(elp_fixture);
-    if diagnostics_enabled.needs_erlang_service() {
-        let file_id = FileId(0);
-        let project_id = db.file_project_id(file_id).unwrap();
-        db.ensure_erlang_service(project_id).unwrap();
-    }
     let host = AnalysisHost { db };
     let analysis = host.analysis();
     for file_id in files {
@@ -382,11 +376,6 @@ pub(crate) fn check_filtered_diagnostics_with_config(
     filter: &dyn Fn(&Diagnostic) -> bool,
 ) {
     let (db, files, diagnostics_enabled) = RootDatabase::with_many_files(elp_fixture);
-    if diagnostics_enabled.needs_erlang_service() {
-        let file_id = FileId(0);
-        let project_id = db.file_project_id(file_id).unwrap();
-        db.ensure_erlang_service(project_id).unwrap();
-    }
     let host = AnalysisHost { db };
     let analysis = host.analysis();
     for file_id in files {
@@ -475,7 +464,7 @@ pub fn check_call_hierarchy(prepare_fixture: &str, incoming_fixture: &str, outgo
 
 fn check_call_hierarchy_prepare(fixture: &str) {
     let trimmed_fixture = trim_indent(fixture);
-    let (analysis, pos, _diagnostics_enabled, _guard, mut annotations) =
+    let (analysis, pos, _diagnostics_enabled, mut annotations) =
         fixture::annotations(trimmed_fixture.as_str());
     let mut navs = analysis.call_hierarchy_prepare(pos).unwrap().unwrap().info;
     assert_eq!(navs.len(), 1);
@@ -491,7 +480,7 @@ fn check_call_hierarchy_prepare(fixture: &str) {
 
 fn check_call_hierarchy_incoming_calls(fixture: &str) {
     let trimmed_fixture = trim_indent(fixture);
-    let (analysis, pos, _diagnostics_enabled, _guard, mut expected) =
+    let (analysis, pos, _diagnostics_enabled, mut expected) =
         fixture::annotations(trimmed_fixture.as_str());
     let incoming_calls = analysis.incoming_calls(pos).unwrap().unwrap();
     let mut actual = Vec::new();
@@ -522,7 +511,7 @@ fn check_call_hierarchy_incoming_calls(fixture: &str) {
 
 fn check_call_hierarchy_outgoing_calls(fixture: &str) {
     let trimmed_fixture = trim_indent(fixture);
-    let (analysis, pos, _diagnostics_enabled, _guard, mut expected) =
+    let (analysis, pos, _diagnostics_enabled, mut expected) =
         fixture::annotations(trimmed_fixture.as_str());
     let outgoing_calls = analysis.outgoing_calls(pos).unwrap().unwrap();
     let mut actual = Vec::new();

crates/ide_db/src/common_test.rs

@@ -111,36 +111,33 @@ impl CommonTestLoader for crate::RootDatabase {
         macros: &[eetf::Term],
         parse_transforms: &[eetf::Term],
     ) -> CommonTestInfo {
-        if let Some(erlang_service) = self.erlang_services.read().get(&project_id).cloned() {
-            let includes = include_path
-                .iter()
-                .map(|path| path.clone().into())
-                .collect();
-            let compile_options = vec![
-                CompileOption::Includes(includes),
-                CompileOption::Macros(macros.to_vec()),
-                CompileOption::ParseTransforms(parse_transforms.to_vec()),
-            ];
-            let should_request_groups = def_map
-                .is_function_exported(&NameArity::new(Name::from_erlang_service("groups"), 0));
-            let request = CTInfoRequest {
-                module: eetf::Atom::from(module.to_string()),
-                src_path,
-                compile_options,
-                should_request_groups,
-            };
-            match erlang_service.ct_info(request, || self.unwind_if_cancelled()) {
-                Ok(result) => match result.all() {
-                    Ok(all) => match result.groups() {
-                        Ok(groups) => CommonTestInfo::Result { all, groups },
-                        Err(err) => CommonTestInfo::ConversionError(err),
-                    },
+        let erlang_service = self.erlang_service_for(project_id);
+        let includes = include_path
+            .iter()
+            .map(|path| path.clone().into())
+            .collect();
+        let compile_options = vec![
+            CompileOption::Includes(includes),
+            CompileOption::Macros(macros.to_vec()),
+            CompileOption::ParseTransforms(parse_transforms.to_vec()),
+        ];
+        let should_request_groups =
+            def_map.is_function_exported(&NameArity::new(Name::from_erlang_service("groups"), 0));
+        let request = CTInfoRequest {
+            module: eetf::Atom::from(module.to_string()),
+            src_path,
+            compile_options,
+            should_request_groups,
+        };
+        match erlang_service.ct_info(request, || self.unwind_if_cancelled()) {
+            Ok(result) => match result.all() {
+                Ok(all) => match result.groups() {
+                    Ok(groups) => CommonTestInfo::Result { all, groups },
                     Err(err) => CommonTestInfo::ConversionError(err),
                 },
-                Err(err) => CommonTestInfo::EvalError(err),
-            }
-        } else {
-            CommonTestInfo::SetupError("No Erlang service".to_string())
+                Err(err) => CommonTestInfo::ConversionError(err),
+            },
+            Err(err) => CommonTestInfo::EvalError(err),
         }
     }
 }