Fix infinite loop when creating a graph for a project with cyclic dependencies

src/bans/graph.rs

@@ -3,7 +3,10 @@ use crate::{DepKind, Kid, Krate};
 use anyhow::{Context, Error};
 use krates::petgraph as pg;
 use semver::Version;
-use std::{collections::HashMap, fmt};
+use std::{
+    collections::{hash_map::Entry, HashMap, HashSet},
+    fmt,
+};
 
 #[derive(Hash, Copy, Clone, PartialEq, Eq)]
 struct Node<'a> {
@@ -201,7 +204,7 @@ pub(crate) fn create_graph(
     // them stand out more in the graph
     for id in &duplicates {
         let dup_node = node_map[id];
-        let mut set = std::collections::HashSet::new();
+        let mut set = HashSet::new();
 
         node_stack.push(dup_node);
 
@@ -210,21 +213,22 @@ pub(crate) fn create_graph(
             let mut iditer = node.repr.splitn(3, ' ');
             let name = iditer.next().unwrap();
 
-            match dupe_nodes.get_mut(name) {
-                Some(v) => {
-                    if !v.contains(&nid) {
-                        v.push(nid);
+            match dupe_nodes.entry(name) {
+                Entry::Occupied(it) => {
+                    let it = it.into_mut();
+                    if !it.contains(&nid) {
+                        it.push(nid);
                     }
                 }
-                None => {
-                    dupe_nodes.insert(name, vec![nid]);
+                Entry::Vacant(it) => {
+                    it.insert(vec![nid]);
                 }
             }
 
             for edge in graph.edges_directed(nid, pg::Direction::Incoming) {
-                set.insert(edge.id());
-
-                node_stack.push(edge.source());
+                if set.insert(edge.id()) {
+                    node_stack.push(edge.source())
+                }
             }
         }
 

tests/bans.rs

@@ -0,0 +1,66 @@
+use cargo_deny::{
+    bans::{
+        self,
+        cfg::{Config, ValidConfig},
+    },
+    diag::KrateSpans,
+    CheckCtx,
+};
+use std::{fs, path::PathBuf, time::Duration};
+
+fn with_test_data(project_name: &str, accept_ctx: impl FnOnce(CheckCtx<'_, ValidConfig>)) {
+    let project_dir = PathBuf::from("./tests/test_data").join(project_name);
+
+    let mut metadata_cmd = krates::cm::MetadataCommand::new();
+    metadata_cmd.current_dir(&project_dir);
+
+    let krates = &krates::Builder::new()
+        .build(metadata_cmd, krates::NoneFilter)
+        .unwrap();
+
+    let spans = KrateSpans::new(krates);
+    let mut files = codespan::Files::new();
+    let spans_id = files.add(project_dir.join("Cargo.lock"), spans.1);
+
+    let config: Config = fs::read_to_string(project_dir.join("deny.toml"))
+        .map(|it| toml::from_str(&it).unwrap())
+        .unwrap_or_default();
+
+    accept_ctx(cargo_deny::CheckCtx {
+        krates,
+        krate_spans: &spans.0,
+        spans_id,
+        cfg: config.validate(spans_id).unwrap(),
+    });
+}
+
+// Covers issue https://github.com/EmbarkStudios/cargo-deny/issues/184
+#[test]
+fn cyclic_dependencies_do_not_cause_infinite_loop() {
+    let (tx, rx) = crossbeam::unbounded();
+
+    let handle = std::thread::spawn(|| {
+        with_test_data("cyclic_dependencies", |check_ctx| {
+            let graph_output = Box::new(|_| Ok(()));
+            bans::check(check_ctx, Some(graph_output), tx);
+        });
+    });
+
+    let timeout_duration = Duration::from_millis(10000);
+    let timeout = crossbeam::after(timeout_duration);
+    loop {
+        crossbeam::select! {
+            recv(rx) -> msg => {
+                if msg.is_err() {
+                    // Yay, the sender was dopped (i.e. check was finished)
+                    break;
+                }
+            }
+            recv(timeout) -> _ => {
+                panic!("Bans check timed out after {:?}", timeout_duration);
+            }
+        }
+    }
+
+    handle.join().unwrap();
+}

tests/test_data/cyclic_dependencies/Cargo.toml

@@ -0,0 +1,2 @@
+[workspace]
+members = ["root", "leaf"]

tests/test_data/cyclic_dependencies/leaf/Cargo.toml

@@ -0,0 +1,9 @@
+[package]
+name = "leaf"
+version = "0.1.0"
+authors = ["veetaha <[email protected]>"]
+edition = "2018"
+
+[dev-dependencies]
+root = { path = "../root" }
+ansi_term = "0.12.1"

tests/test_data/cyclic_dependencies/leaf/src/lib.rs

@@ -0,0 +1 @@
+pub fn leaf() {}

tests/test_data/cyclic_dependencies/root/Cargo.toml

@@ -0,0 +1,9 @@
+[package]
+name = "root"
+version = "0.1.0"
+authors = ["veetaha <[email protected]>"]
+edition = "2018"
+
+[dependencies]
+ansi_term = "0.11.0"
+leaf = { path = "../leaf" }

tests/test_data/cyclic_dependencies/root/src/lib.rs

@@ -0,0 +1 @@
+pub fn root() {}

tests/test_data/cyclic_dependencies/root/src/main.rs

@@ -0,0 +1 @@
+fn main() {}