From 71efd950d17faabc4025acb460c4000bf97978f7 Mon Sep 17 00:00:00 2001 From: Ralf Jung <post@ralfj.de> Date: Sun, 25 Jul 2021 14:08:12 +0200 Subject: [PATCH 1/5] also ignore 'thread leaks' with -Zmiri-ignore-leaks --- README.md | 3 ++- src/eval.rs | 6 +++++ src/thread.rs | 21 +++++++++------- .../libc_pthread_create_main_terminate.rs | 4 ++-- tests/run-pass/threadleak_ignored.rs | 24 +++++++++++++++++++ tests/run-pass/threadleak_ignored.stderr | 3 +++ 6 files changed, 49 insertions(+), 12 deletions(-) create mode 100644 tests/run-pass/threadleak_ignored.rs create mode 100644 tests/run-pass/threadleak_ignored.stderr diff --git a/README.md b/README.md index cbac48db5e..7cd802762b 100644 --- a/README.md +++ b/README.md @@ -230,7 +230,8 @@ environment variable: the host so that it cannot be accessed by the program. Can be used multiple times to exclude several variables. On Windows, the `TERM` environment variable is excluded by default. -* `-Zmiri-ignore-leaks` disables the memory leak checker. +* `-Zmiri-ignore-leaks` disables the memory leak checker, and also allows some + remaining threads to exist when the main thread exits. * `-Zmiri-measureme=<name>` enables `measureme` profiling for the interpreted program. This can be used to find which parts of your program are executing slowly under Miri. The profile is written out to a file with the prefix `<name>`, and can be processed diff --git a/src/eval.rs b/src/eval.rs index ae9ff9c1f5..9531a2d78a 100644 --- a/src/eval.rs +++ b/src/eval.rs @@ -300,6 +300,12 @@ pub fn eval_main<'tcx>(tcx: TyCtxt<'tcx>, main_id: DefId, config: MiriConfig) -> match res { Ok(return_code) => { if !ignore_leaks { + // Check for thread leaks. + if !ecx.have_all_terminated() { + tcx.sess.err("the main thread terminated without waiting for all remaining threads"); + return None; + } + // Check for memory leaks. info!("Additonal static roots: {:?}", ecx.machine.static_roots); let leaks = ecx.memory.leak_report(&ecx.machine.static_roots); if leaks != 0 { diff --git a/src/thread.rs b/src/thread.rs index de8e41224b..a5deceb6e7 100644 --- a/src/thread.rs +++ b/src/thread.rs @@ -302,6 +302,11 @@ impl<'mir, 'tcx: 'mir> ThreadManager<'mir, 'tcx> { self.threads[thread_id].state == ThreadState::Terminated } + /// Have all threads terminated? + fn have_all_terminated(&self) -> bool { + self.threads.iter().all(|thread| thread.state == ThreadState::Terminated) + } + /// Enable the thread for execution. The thread must be terminated. fn enable_thread(&mut self, thread_id: ThreadId) { assert!(self.has_terminated(thread_id)); @@ -491,15 +496,7 @@ impl<'mir, 'tcx: 'mir> ThreadManager<'mir, 'tcx> { // If we get here again and the thread is *still* terminated, there are no more dtors to run. if self.threads[MAIN_THREAD].state == ThreadState::Terminated { // The main thread terminated; stop the program. - if self.threads.iter().any(|thread| thread.state != ThreadState::Terminated) { - // FIXME: This check should be either configurable or just emit - // a warning. For example, it seems normal for a program to - // terminate without waiting for its detached threads to - // terminate. However, this case is not trivial to support - // because we also probably do not want to consider the memory - // owned by these threads as leaked. - throw_unsup_format!("the main thread terminated without waiting for other threads"); - } + // We do *not* run TLS dtors of remaining threads, which seems to match rustc behavior. return Ok(SchedulingAction::Stop); } // This thread and the program can keep going. @@ -645,6 +642,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx this.machine.threads.has_terminated(thread_id) } + #[inline] + fn have_all_terminated(&self) -> bool { + let this = self.eval_context_ref(); + this.machine.threads.have_all_terminated() + } + #[inline] fn enable_thread(&mut self, thread_id: ThreadId) { let this = self.eval_context_mut(); diff --git a/tests/compile-fail/concurrency/libc_pthread_create_main_terminate.rs b/tests/compile-fail/concurrency/libc_pthread_create_main_terminate.rs index 35ee03242d..9b576bbb08 100644 --- a/tests/compile-fail/concurrency/libc_pthread_create_main_terminate.rs +++ b/tests/compile-fail/concurrency/libc_pthread_create_main_terminate.rs @@ -1,5 +1,5 @@ // ignore-windows: No libc on Windows -// error-pattern: unsupported operation: the main thread terminated without waiting for other threads +// error-pattern: the main thread terminated without waiting for all remaining threads // Check that we terminate the program when the main thread terminates. @@ -10,7 +10,7 @@ extern crate libc; use std::{mem, ptr}; extern "C" fn thread_start(_null: *mut libc::c_void) -> *mut libc::c_void { - ptr::null_mut() + loop {} } fn main() { diff --git a/tests/run-pass/threadleak_ignored.rs b/tests/run-pass/threadleak_ignored.rs new file mode 100644 index 0000000000..14a7449f33 --- /dev/null +++ b/tests/run-pass/threadleak_ignored.rs @@ -0,0 +1,24 @@ +// compile-flags: -Zmiri-ignore-leaks + +//! Test that leaking threads works, and that their destructors are not executed. + +use std::cell::RefCell; + +struct LoudDrop(i32); +impl Drop for LoudDrop { + fn drop(&mut self) { + eprintln!("Dropping {}", self.0); + } +} + +thread_local! { + static X: RefCell<Option<LoudDrop>> = RefCell::new(None); +} + +fn main() { + X.with(|x| *x.borrow_mut() = Some(LoudDrop(0))); + + let _detached = std::thread::spawn(|| { + X.with(|x| *x.borrow_mut() = Some(LoudDrop(1))); + }); +} diff --git a/tests/run-pass/threadleak_ignored.stderr b/tests/run-pass/threadleak_ignored.stderr new file mode 100644 index 0000000000..aa03751185 --- /dev/null +++ b/tests/run-pass/threadleak_ignored.stderr @@ -0,0 +1,3 @@ +warning: thread support is experimental and incomplete: weak memory effects are not emulated. + +Dropping 0 From df9d481989dee1dcbe9a3f886dcbb030deb2144b Mon Sep 17 00:00:00 2001 From: Ralf Jung <post@ralfj.de> Date: Sun, 25 Jul 2021 14:19:57 +0200 Subject: [PATCH 2/5] tell users how to disable the leak check --- src/eval.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/eval.rs b/src/eval.rs index 9531a2d78a..e559dfedf9 100644 --- a/src/eval.rs +++ b/src/eval.rs @@ -302,7 +302,10 @@ pub fn eval_main<'tcx>(tcx: TyCtxt<'tcx>, main_id: DefId, config: MiriConfig) -> if !ignore_leaks { // Check for thread leaks. if !ecx.have_all_terminated() { - tcx.sess.err("the main thread terminated without waiting for all remaining threads"); + tcx.sess.err( + "the main thread terminated without waiting for all remaining threads", + ); + tcx.sess.note_without_error("pass `-Zmiri-ignore-leaks` to disable this check"); return None; } // Check for memory leaks. @@ -310,6 +313,7 @@ pub fn eval_main<'tcx>(tcx: TyCtxt<'tcx>, main_id: DefId, config: MiriConfig) -> let leaks = ecx.memory.leak_report(&ecx.machine.static_roots); if leaks != 0 { tcx.sess.err("the evaluated program leaked memory"); + tcx.sess.note_without_error("pass `-Zmiri-ignore-leaks` to disable this check"); // Ignore the provided return code - let the reported error // determine the return code. return None; From 679d10f98b765294aefbce61b525de407fc2cea4 Mon Sep 17 00:00:00 2001 From: Ralf Jung <post@ralfj.de> Date: Sun, 25 Jul 2021 14:38:02 +0200 Subject: [PATCH 3/5] no concurrency on windows --- tests/run-pass/threadleak_ignored.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/run-pass/threadleak_ignored.rs b/tests/run-pass/threadleak_ignored.rs index 14a7449f33..840fbc1ebc 100644 --- a/tests/run-pass/threadleak_ignored.rs +++ b/tests/run-pass/threadleak_ignored.rs @@ -1,3 +1,4 @@ +// ignore-windows: Concurrency on Windows is not supported yet. // compile-flags: -Zmiri-ignore-leaks //! Test that leaking threads works, and that their destructors are not executed. From 66aa3d0247fd47051077f005d2d1462316c83fe1 Mon Sep 17 00:00:00 2001 From: Ralf Jung <post@ralfj.de> Date: Mon, 26 Jul 2021 21:54:28 +0200 Subject: [PATCH 4/5] make the loop infinite --- tests/run-pass/threadleak_ignored.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/run-pass/threadleak_ignored.rs b/tests/run-pass/threadleak_ignored.rs index 840fbc1ebc..4fc52cdb8d 100644 --- a/tests/run-pass/threadleak_ignored.rs +++ b/tests/run-pass/threadleak_ignored.rs @@ -21,5 +21,6 @@ fn main() { let _detached = std::thread::spawn(|| { X.with(|x| *x.borrow_mut() = Some(LoudDrop(1))); + loop {} }); } From 78bcd12b17ae32fb74815cb2908a5149d5713415 Mon Sep 17 00:00:00 2001 From: Ralf Jung <post@ralfj.de> Date: Tue, 27 Jul 2021 14:05:37 +0200 Subject: [PATCH 5/5] make sure we only terminate main thread once TLS is initialized --- tests/run-pass/threadleak_ignored.rs | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/tests/run-pass/threadleak_ignored.rs b/tests/run-pass/threadleak_ignored.rs index 4fc52cdb8d..7bb51d2dea 100644 --- a/tests/run-pass/threadleak_ignored.rs +++ b/tests/run-pass/threadleak_ignored.rs @@ -18,9 +18,20 @@ thread_local! { fn main() { X.with(|x| *x.borrow_mut() = Some(LoudDrop(0))); + + // Set up a channel so that we can learn when the other thread initialized `X` + // (so that we are sure there is something to drop). + let (send, recv) = std::sync::mpsc::channel::<()>(); - let _detached = std::thread::spawn(|| { + let _detached = std::thread::spawn(move || { X.with(|x| *x.borrow_mut() = Some(LoudDrop(1))); + send.send(()).unwrap(); + std::thread::yield_now(); loop {} }); + + std::thread::yield_now(); + + // Wait until child thread has initialized its `X`. + let () = recv.recv().unwrap(); }