From 15752bc9da5e245cfb238a7c02803b08d49042ca Mon Sep 17 00:00:00 2001 From: Shunsuke Shibayama Date: Mon, 9 Dec 2024 23:45:53 +0900 Subject: [PATCH] fix: `SharedPromise::join` deadlock --- Cargo.toml | 4 +-- crates/erg_common/Cargo.toml | 3 +- crates/erg_compiler/Cargo.toml | 3 +- crates/erg_compiler/module/promise.rs | 46 +++++++++++++++++---------- crates/erg_linter/Cargo.toml | 1 + crates/erg_parser/Cargo.toml | 3 +- doc/EN/dev_guide/build_features.md | 4 +-- doc/JA/dev_guide/build_features.md | 4 +-- 8 files changed, 43 insertions(+), 25 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 1293ac44f..1e2851ea5 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -31,6 +31,7 @@ repository = "https://github.com/erg-lang/erg" homepage = "https://erg-lang.org/" [features] +default = ["parallel"] # when "debug" feature is turned on, that of the following crates will also be turned on. debug = ["erg_common/debug", "erg_parser/debug", "erg_compiler/debug", "erg_linter/debug"] # "els/debug" backtrace = ["erg_common/backtrace", "els/backtrace"] @@ -69,9 +70,8 @@ gal = ["erg_common/gal", "erg_compiler/gal"] els = ["erg_common/els", "erg_compiler/els", "dep:els"] full-repl = ["erg_common/full-repl"] full = ["els", "full-repl", "unicode", "pretty"] -experimental = ["erg_common/experimental", "erg_parser/experimental", "erg_compiler/experimental", "erg_linter/experimental", "parallel"] +experimental = ["erg_common/experimental", "erg_parser/experimental", "erg_compiler/experimental", "erg_linter/experimental"] log-level-error = ["erg_common/log-level-error", "erg_parser/log-level-error", "erg_compiler/log-level-error", "erg_linter/log-level-error"] -# The parallelizing compiler was found to contain a bug that caused it to hang in complex dependencies, so it is disabled by default. parallel = ["erg_common/parallel", "erg_parser/parallel", "erg_compiler/parallel", "erg_linter/parallel"] [workspace.dependencies] diff --git a/crates/erg_common/Cargo.toml b/crates/erg_common/Cargo.toml index f9f61c7c6..ebef03e16 100644 --- a/crates/erg_common/Cargo.toml +++ b/crates/erg_common/Cargo.toml @@ -10,6 +10,7 @@ repository.workspace = true homepage.workspace = true [features] +default = ["parallel"] debug = ["dep:backtrace-on-stack-overflow", "dep:w-boson"] backtrace = ["dep:backtrace-on-stack-overflow", "dep:w-boson"] japanese = [] @@ -23,7 +24,7 @@ py_compat = [] gal = [] no_std = [] full-repl = ["dep:crossterm"] -experimental = ["parallel"] +experimental = [] pylib = ["dep:pyo3"] log-level-error = [] parallel = [] diff --git a/crates/erg_compiler/Cargo.toml b/crates/erg_compiler/Cargo.toml index a5c4c8ec9..c789218df 100644 --- a/crates/erg_compiler/Cargo.toml +++ b/crates/erg_compiler/Cargo.toml @@ -11,6 +11,7 @@ repository.workspace = true homepage.workspace = true [features] +default = ["parallel"] # when "debug" feature is turned on, that of parser will also be turned on. debug = ["erg_common/debug", "erg_parser/debug"] japanese = ["erg_common/japanese", "erg_parser/japanese"] @@ -39,7 +40,7 @@ gal = ["erg_common/gal"] els = ["erg_common/els"] no_std = ["erg_common/no_std"] full-repl = ["erg_common/full-repl"] -experimental = ["erg_common/experimental", "erg_parser/experimental", "parallel"] +experimental = ["erg_common/experimental", "erg_parser/experimental"] pylib = ["dep:pyo3", "erg_common/pylib", "erg_parser/pylib"] pylib_compiler = ["pylib"] log-level-error = ["erg_common/log-level-error", "erg_parser/log-level-error"] diff --git a/crates/erg_compiler/module/promise.rs b/crates/erg_compiler/module/promise.rs index 268ad978f..db84b9543 100644 --- a/crates/erg_compiler/module/promise.rs +++ b/crates/erg_compiler/module/promise.rs @@ -17,7 +17,9 @@ pub enum Promise { parent: ThreadId, handle: JoinHandle<()>, }, - Joining, + Joining { + id: ThreadId, + }, Joined, } @@ -27,7 +29,7 @@ impl fmt::Display for Promise { Self::Running { handle, .. } => { write!(f, "running on thread {:?}", handle.thread().id()) } - Self::Joining => write!(f, "joining"), + Self::Joining { id } => write!(f, "joining (thread {id:?})"), Self::Joined => write!(f, "joined"), } } @@ -44,36 +46,40 @@ impl Promise { /// can be joined if `true` pub fn is_finished(&self) -> bool { match self { - Self::Joined => true, - Self::Joining => false, + Self::Joined { .. } => true, + Self::Joining { .. } => false, Self::Running { handle, .. } => handle.is_finished(), } } pub const fn is_joined(&self) -> bool { - matches!(self, Self::Joined) + matches!(self, Self::Joined { .. }) } pub const fn is_joining(&self) -> bool { - matches!(self, Self::Joining) + matches!(self, Self::Joining { .. }) } pub fn thread_id(&self) -> Option { match self { - Self::Joined | Self::Joining => None, + Self::Joined => None, + Self::Joining { id } => Some(*id), Self::Running { handle, .. } => Some(handle.thread().id()), } } pub fn parent_thread_id(&self) -> Option { match self { - Self::Joined | Self::Joining => None, + Self::Joined { .. } | Self::Joining { .. } => None, Self::Running { parent, .. } => Some(*parent), } } pub fn take(&mut self) -> Self { - std::mem::replace(self, Self::Joining) + let joining = Self::Joining { + id: self.thread_id().unwrap(), + }; + std::mem::replace(self, joining) } } @@ -197,7 +203,12 @@ impl SharedPromises { return Ok(()); } } - while let Some(Promise::Joining) | None = self.promises.borrow().get(path) { + // prevent deadlock + if self.thread_id(path).is_some_and(|id| id == current().id()) { + *self.promises.borrow_mut().get_mut(path).unwrap() = Promise::Joined; + return Ok(()); + } + while let Some(Promise::Joining { .. }) | None = self.promises.borrow().get(path) { safe_yield(); } if self.is_joined(path) { @@ -209,10 +220,6 @@ impl SharedPromises { self.wait_until_finished(path); return Ok(()); }; - if handle.thread().id() == current().id() { - *self.promises.borrow_mut().get_mut(path).unwrap() = Promise::Joined; - return Ok(()); - } let res = handle.join(); *self.promises.borrow_mut().get_mut(path).unwrap() = Promise::Joined; res @@ -239,6 +246,13 @@ impl SharedPromises { } } + pub fn thread_id(&self, path: &NormalizedPathBuf) -> Option { + self.promises + .borrow() + .get(path) + .and_then(|promise| promise.thread_id()) + } + pub fn progress(&self) -> Progress { let mut total = 0; let mut running = 0; @@ -246,8 +260,8 @@ impl SharedPromises { for promise in self.promises.borrow().values() { match promise { Promise::Running { .. } => running += 1, - Promise::Joining => finished += 1, - Promise::Joined => finished += 1, + Promise::Joining { .. } => finished += 1, + Promise::Joined { .. } => finished += 1, } total += 1; } diff --git a/crates/erg_linter/Cargo.toml b/crates/erg_linter/Cargo.toml index be3bd31be..7d9a1efa0 100644 --- a/crates/erg_linter/Cargo.toml +++ b/crates/erg_linter/Cargo.toml @@ -10,6 +10,7 @@ repository.workspace = true homepage.workspace = true [features] +default = ["parallel"] debug = ["erg_common/debug", "erg_parser/debug", "erg_compiler/debug"] backtrace = ["erg_common/backtrace"] japanese = [ diff --git a/crates/erg_parser/Cargo.toml b/crates/erg_parser/Cargo.toml index 1d240e958..5f18bdffe 100644 --- a/crates/erg_parser/Cargo.toml +++ b/crates/erg_parser/Cargo.toml @@ -10,6 +10,7 @@ repository.workspace = true homepage.workspace = true [features] +default = ["parallel"] debug = ["erg_common/debug"] japanese = ["erg_common/japanese"] simplified_chinese = ["erg_common/simplified_chinese"] @@ -17,7 +18,7 @@ traditional_chinese = ["erg_common/traditional_chinese"] unicode = ["erg_common/unicode"] pretty = ["erg_common/pretty"] large_thread = ["erg_common/large_thread"] -experimental = ["erg_common/experimental", "parallel"] +experimental = ["erg_common/experimental"] pylib = ["dep:pyo3", "erg_common/pylib"] pylib_parser = ["pylib"] log-level-error = ["erg_common/log-level-error"] diff --git a/doc/EN/dev_guide/build_features.md b/doc/EN/dev_guide/build_features.md index 52a4df21e..3c27ae566 100644 --- a/doc/EN/dev_guide/build_features.md +++ b/doc/EN/dev_guide/build_features.md @@ -43,7 +43,7 @@ Enable Python-compatible mode, which makes parts of the APIs and syntax compatib ## experimental -Enable experimental features (contains `parallel`). +Enable experimental features. ## log-level-error @@ -51,4 +51,4 @@ Only display error logs. ## parallel -Enable compiler parallelization. Unstable feature. +Enable compiler parallelization. Default is enabled. diff --git a/doc/JA/dev_guide/build_features.md b/doc/JA/dev_guide/build_features.md index 27e72bd6d..768da6b14 100644 --- a/doc/JA/dev_guide/build_features.md +++ b/doc/JA/dev_guide/build_features.md @@ -45,7 +45,7 @@ Python互換モードを有効にする。APIや文法の一部がPythonと互 ## experimental -実験的な機能を有効にする。`parallel`も有効化される。 +実験的な機能を有効にする。 ## log-level-error @@ -53,4 +53,4 @@ Python互換モードを有効にする。APIや文法の一部がPythonと互 ## parallel -コンパイラの並列化を有効にする。不安定機能。 +コンパイラの並列化を有効にする。デフォルトは有効。