Auto merge of #2397 - RalfJung:cleanup, r=RalfJung

only do env var cleanup if all threads have stopped

Hopefully fixes #2396

Author: bors

src/concurrency/data_race.rs

@@ -436,45 +436,6 @@ impl MemoryCellClocks {
 /// Evaluation context extensions.
 impl<'mir, 'tcx: 'mir> EvalContextExt<'mir, 'tcx> for MiriEvalContext<'mir, 'tcx> {}
 pub trait EvalContextExt<'mir, 'tcx: 'mir>: MiriEvalContextExt<'mir, 'tcx> {
-    /// Temporarily allow data-races to occur. This should only be used in
-    /// one of these cases:
-    /// - One of the appropriate `validate_atomic` functions will be called to
-    /// to treat a memory access as atomic.
-    /// - The memory being accessed should be treated as internal state, that
-    /// cannot be accessed by the interpreted program.
-    /// - Execution of the interpreted program execution has halted.
-    #[inline]
-    fn allow_data_races_ref<R>(&self, op: impl FnOnce(&MiriEvalContext<'mir, 'tcx>) -> R) -> R {
-        let this = self.eval_context_ref();
-        if let Some(data_race) = &this.machine.data_race {
-            data_race.ongoing_action_data_race_free.set(true);
-        }
-        let result = op(this);
-        if let Some(data_race) = &this.machine.data_race {
-            data_race.ongoing_action_data_race_free.set(false);
-        }
-        result
-    }
-
-    /// Same as `allow_data_races_ref`, this temporarily disables any data-race detection and
-    /// so should only be used for atomic operations or internal state that the program cannot
-    /// access.
-    #[inline]
-    fn allow_data_races_mut<R>(
-        &mut self,
-        op: impl FnOnce(&mut MiriEvalContext<'mir, 'tcx>) -> R,
-    ) -> R {
-        let this = self.eval_context_mut();
-        if let Some(data_race) = &this.machine.data_race {
-            data_race.ongoing_action_data_race_free.set(true);
-        }
-        let result = op(this);
-        if let Some(data_race) = &this.machine.data_race {
-            data_race.ongoing_action_data_race_free.set(false);
-        }
-        result
-    }
-
     /// Atomic variant of read_scalar_at_offset.
     fn read_scalar_at_offset_atomic(
         &self,
@@ -1044,6 +1005,45 @@ impl VClockAlloc {
 
 impl<'mir, 'tcx: 'mir> EvalContextPrivExt<'mir, 'tcx> for MiriEvalContext<'mir, 'tcx> {}
 trait EvalContextPrivExt<'mir, 'tcx: 'mir>: MiriEvalContextExt<'mir, 'tcx> {
+    /// Temporarily allow data-races to occur. This should only be used in
+    /// one of these cases:
+    /// - One of the appropriate `validate_atomic` functions will be called to
+    /// to treat a memory access as atomic.
+    /// - The memory being accessed should be treated as internal state, that
+    /// cannot be accessed by the interpreted program.
+    /// - Execution of the interpreted program execution has halted.
+    #[inline]
+    fn allow_data_races_ref<R>(&self, op: impl FnOnce(&MiriEvalContext<'mir, 'tcx>) -> R) -> R {
+        let this = self.eval_context_ref();
+        if let Some(data_race) = &this.machine.data_race {
+            data_race.ongoing_action_data_race_free.set(true);
+        }
+        let result = op(this);
+        if let Some(data_race) = &this.machine.data_race {
+            data_race.ongoing_action_data_race_free.set(false);
+        }
+        result
+    }
+
+    /// Same as `allow_data_races_ref`, this temporarily disables any data-race detection and
+    /// so should only be used for atomic operations or internal state that the program cannot
+    /// access.
+    #[inline]
+    fn allow_data_races_mut<R>(
+        &mut self,
+        op: impl FnOnce(&mut MiriEvalContext<'mir, 'tcx>) -> R,
+    ) -> R {
+        let this = self.eval_context_mut();
+        if let Some(data_race) = &this.machine.data_race {
+            data_race.ongoing_action_data_race_free.set(true);
+        }
+        let result = op(this);
+        if let Some(data_race) = &this.machine.data_race {
+            data_race.ongoing_action_data_race_free.set(false);
+        }
+        result
+    }
+
     /// Generic atomic operation implementation
     fn validate_atomic_op<A: Debug + Copy>(
         &self,

src/eval.rs

@@ -363,13 +363,12 @@ pub fn eval_entry<'tcx>(
         panic::resume_unwind(panic_payload)
     });
 
-    // Machine cleanup.
-    // Execution of the program has halted so any memory access we do here
-    // cannot produce a real data race. If we do not do something to disable
-    // data race detection here, some uncommon combination of errors will
-    // cause a data race to be detected:
-    // https://github.com/rust-lang/miri/issues/2020
-    ecx.allow_data_races_mut(|ecx| EnvVars::cleanup(ecx).unwrap());
+    // Machine cleanup. Only do this if all threads have terminated; threads that are still running
+    // might cause data races (https://github.com/rust-lang/miri/issues/2020) or Stacked Borrows
+    // errors (https://github.com/rust-lang/miri/issues/2396) if we deallocate here.
+    if ecx.have_all_terminated() {
+        EnvVars::cleanup(&mut ecx).unwrap();
+    }
 
     // Process the result.
     match res {