Skip to content

Commit 9ff6e14

Browse files
committed
Fix race condition when emitting stored diagnostics
# Conflicts: # src/librustc/dep_graph/graph.rs
1 parent 3dc1efd commit 9ff6e14

File tree

1 file changed

+63
-21
lines changed

1 file changed

+63
-21
lines changed

src/librustc/dep_graph/graph.rs

+63-21
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
// option. This file may not be copied, modified, or distributed
99
// except according to those terms.
1010

11-
use errors::DiagnosticBuilder;
11+
use errors::{Diagnostic, DiagnosticBuilder};
1212
use rustc_data_structures::stable_hasher::{HashStable, StableHasher};
1313
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
1414
use rustc_data_structures::indexed_vec::{Idx, IndexVec};
@@ -19,6 +19,7 @@ use std::hash::Hash;
1919
use std::collections::hash_map::Entry;
2020
use ty::{self, TyCtxt};
2121
use util::common::{ProfileQueriesMsg, profq_msg};
22+
use parking_lot::{Mutex, Condvar};
2223

2324
use ich::{StableHashingContext, StableHashingContextProvider, Fingerprint};
2425

@@ -70,6 +71,12 @@ struct DepGraphData {
7071

7172
colors: DepNodeColorMap,
7273

74+
/// A set of loaded diagnostics which has been emitted.
75+
emitted_diagnostics: Mutex<FxHashSet<DepNodeIndex>>,
76+
77+
/// Used to wait for diagnostics to be emitted.
78+
emitted_diagnostics_cond_var: Condvar,
79+
7380
/// When we load, there may be `.o` files, cached mir, or other such
7481
/// things available to us. If we find that they are not dirty, we
7582
/// load the path to the file storing those work-products here into
@@ -93,6 +100,8 @@ impl DepGraph {
93100
previous_work_products: prev_work_products,
94101
dep_node_debug: Default::default(),
95102
current: Lock::new(CurrentDepGraph::new(prev_graph_node_count)),
103+
emitted_diagnostics: Default::default(),
104+
emitted_diagnostics_cond_var: Condvar::new(),
96105
previous: prev_graph,
97106
colors: DepNodeColorMap::new(prev_graph_node_count),
98107
loaded_from_cache: Default::default(),
@@ -711,28 +720,18 @@ impl DepGraph {
711720
};
712721

713722
// ... emitting any stored diagnostic ...
714-
if did_allocation {
715-
// Only the thread which did the allocation emits the error messages
716723

717-
// FIXME: Ensure that these are printed before returning for all threads.
718-
// Currently threads where did_allocation = false can continue on
719-
// and emit other diagnostics before these diagnostics are emitted.
720-
// Such diagnostics should be emitted after these.
721-
// See https://github.com/rust-lang/rust/issues/48685
722-
let diagnostics = tcx.queries.on_disk_cache
723-
.load_diagnostics(tcx, prev_dep_node_index);
724+
let diagnostics = tcx.queries.on_disk_cache
725+
.load_diagnostics(tcx, prev_dep_node_index);
724726

725-
if diagnostics.len() > 0 {
726-
let handle = tcx.sess.diagnostic();
727-
728-
// Promote the previous diagnostics to the current session.
729-
tcx.queries.on_disk_cache
730-
.store_diagnostics(dep_node_index, diagnostics.clone());
731-
732-
for diagnostic in diagnostics {
733-
DiagnosticBuilder::new_diagnostic(handle, diagnostic).emit();
734-
}
735-
}
727+
if unlikely!(diagnostics.len() > 0) {
728+
self.emit_diagnostics(
729+
tcx,
730+
data,
731+
dep_node_index,
732+
did_allocation,
733+
diagnostics
734+
);
736735
}
737736

738737
// ... and finally storing a "Green" entry in the color map.
@@ -748,6 +747,49 @@ impl DepGraph {
748747
Some(dep_node_index)
749748
}
750749

750+
/// Atomically emits some loaded diagnotics assuming that this only gets called with
751+
/// did_allocation set to true on one thread
752+
#[cold]
753+
#[inline(never)]
754+
fn emit_diagnostics<'tcx>(
755+
&self,
756+
tcx: TyCtxt<'_, 'tcx, 'tcx>,
757+
data: &DepGraphData,
758+
dep_node_index: DepNodeIndex,
759+
did_allocation: bool,
760+
diagnostics: Vec<Diagnostic>,
761+
) {
762+
if did_allocation || !cfg!(parallel_queries) {
763+
// Only the thread which did the allocation emits the error messages
764+
let handle = tcx.sess.diagnostic();
765+
766+
// Promote the previous diagnostics to the current session.
767+
tcx.queries.on_disk_cache
768+
.store_diagnostics(dep_node_index, diagnostics.clone());
769+
770+
for diagnostic in diagnostics {
771+
DiagnosticBuilder::new_diagnostic(handle, diagnostic).emit();
772+
}
773+
774+
#[cfg(parallel_queries)]
775+
{
776+
// Mark the diagnostics and emitted and wake up waiters
777+
data.emitted_diagnostics.lock().insert(dep_node_index);
778+
data.emitted_diagnostics_cond_var.notify_all();
779+
}
780+
} else {
781+
// The other threads will wait for the diagnostics to be emitted
782+
783+
let mut emitted_diagnostics = data.emitted_diagnostics.lock();
784+
loop {
785+
if emitted_diagnostics.contains(&dep_node_index) {
786+
break;
787+
}
788+
data.emitted_diagnostics_cond_var.wait(&mut emitted_diagnostics);
789+
}
790+
}
791+
}
792+
751793
// Returns true if the given node has been marked as green during the
752794
// current compilation session. Used in various assertions
753795
pub fn is_green(&self, dep_node: &DepNode) -> bool {

0 commit comments

Comments
 (0)