Skip to content

Commit

Permalink
fix: SharedPromises::join deadlock (2)
Browse files Browse the repository at this point in the history
  • Loading branch information
mtshiba committed Dec 14, 2024
1 parent 867510d commit 61e91c2
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 11 deletions.
5 changes: 4 additions & 1 deletion crates/erg_compiler/module/graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,8 +127,11 @@ impl ModuleGraph {
/// ```erg
/// # a.er
/// b = import "b"
/// # -> a: child, b: parent
/// # b.er
/// c = import "c"
/// # -> ancestors(a) == {b, c}
/// ```
/// -> a: child, b: parent<br>
/// O(N)
pub fn ancestors<'p>(&'p self, path: &'p NormalizedPathBuf) -> Set<&'p NormalizedPathBuf> {
let mut ancestors = set! {};
Expand Down
29 changes: 19 additions & 10 deletions crates/erg_compiler/module/promise.rs
Original file line number Diff line number Diff line change
Expand Up @@ -182,13 +182,10 @@ impl SharedPromises {
if !self.graph.entries().contains(path) {
return Err(Box::new(format!("not registered: {path}")));
}
// prevent deadlock
if self.thread_id(path).is_some_and(|id| id == current().id()) {
return Ok(());
}
if self.graph.ancestors(path).contains(&self.root) || path == &self.root {
// cycle detected, `self.root` must not in the dependencies
// Erg analysis processes never join ancestor threads (although joining ancestors itself is allowed in Rust)
let current = self.current_path();
if self.graph.ancestors(path).contains(&current) || path == &current {
// cycle detected, `current` must not in the dependencies
// Erg analysis processes never join themselves / ancestor threads
// self.wait_until_finished(path);
return Ok(());
}
Expand All @@ -198,12 +195,14 @@ impl SharedPromises {
}
// Suppose A depends on B and C, and B depends on C.
// In this case, B must join C before A joins C. Otherwise, a deadlock will occur.
// C.children() == {A, B}
let children = self.graph.children(path);
for child in children.iter() {
if child == &self.root {
if child == &current {
continue;
} else if self.graph.depends_on(&self.root, child) {
self.wait_until_finished(path);
} else if self.graph.depends_on(&current, child) {
// A.depends_on(B) => A.wait_until_finished(B)
self.wait_until_finished(child);
return Ok(());
}
}
Expand Down Expand Up @@ -252,6 +251,16 @@ impl SharedPromises {
.and_then(|promise| promise.thread_id())
}

pub fn current_path(&self) -> NormalizedPathBuf {
let cur_id = current().id();
for (path, promise) in self.promises.borrow().iter() {
if promise.thread_id() == Some(cur_id) {
return path.clone();
}
}
self.root.clone()
}

pub fn progress(&self) -> Progress {
let mut total = 0;
let mut running = 0;
Expand Down

0 comments on commit 61e91c2

Please sign in to comment.