Auto merge of #570 - Mark-Simulacrum:fix-stall, r=pietroalbini

Fix stall on failure to mark as failed

The root cause of this bug hasn't yet been tracked down, but the additional logging added in the first and second commits should help track it down in the future as well. It seems pretty likely that there are more cases where bailing early leads to problems (as deletion from the graph is extremely necessary but seems easily missed), but hopefully the added logging will help reduce debugging time in the future. A more exhaustive fix here seems difficult with the current abstractions in the code, so would need more thought - my hope is that the bugs we may have can be squashed out pretty quickly with how many runs we have regularly through crater, and ultimately the rewrite avoided (as it has its own likelihood of adding bugs).

Author: bors

src/runner/graph.rs

@@ -230,16 +230,24 @@ impl TasksGraph {
             self.mark_as_failed(child, ex, db, state, config, error, result, worker)?;
         }
 
-        match self.graph[node] {
+        // We need to mark_as_completed the node here (if it's a task),
+        // otherwise we'll later get stuck as the node is still considered
+        // running (but has actually failed).
+        let res = match self.graph[node] {
             Node::Task { ref task, .. } => {
                 log::debug!("marking task {:?} as failed", task);
-                task.mark_as_failed(ex, db, state, config, error, result)?
+                let res = task.mark_as_failed(ex, db, state, config, error, result);
+                if let Err(err) = &res {
+                    log::debug!("marking task {:?} as failed, failed: {:?}", task, err);
+                }
+                res
             }
             Node::CrateCompleted | Node::Root => return Ok(()),
-        }
+        };
 
         self.mark_as_completed(node);
-        Ok(())
+
+        res
     }
 
     pub(super) fn pending_crates_count(&self) -> usize {

src/runner/mod.rs

@@ -101,10 +101,17 @@ pub fn run_ex<DB: WriteResults + Sync>(
         let mut threads = Vec::new();
 
         for worker in &workers {
-            let join = scope
-                .builder()
-                .name(worker.name().into())
-                .spawn(move || worker.run())?;
+            let join =
+                scope
+                    .builder()
+                    .name(worker.name().into())
+                    .spawn(move || match worker.run() {
+                        Ok(()) => Ok(()),
+                        Err(r) => {
+                            log::warn!("worker {} failed: {:?}", worker.name(), r);
+                            Err(r)
+                        }
+                    })?;
             threads.push(join);
         }
         let disk_watcher_thread =