Avoid a panic in set_output_capture in the default panic handler
#122882
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
380964c to
c2e5ee4
Compare
|
@bors r+ |
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Apr 11, 2024
…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#122882 (Avoid a panic in `set_output_capture` in the default panic handler) - rust-lang#123523 (Account for trait/impl difference when suggesting changing argument from ref to mut ref) - rust-lang#123744 (Silence `unused_imports` for redundant imports) - rust-lang#123784 (Replace `document.write` with `document.head.insertAdjacent`) - rust-lang#123798 (Avoid invalid socket address in length calculation) - rust-lang#123804 (Stop using `HirId` for fn-like parents since closures are not `OwnerNode`s) - rust-lang#123806 (Panic on overflow in `BorrowedCursor::advance`) - rust-lang#123820 (Add my former address to .mailmap) r? `@ghost` `@rustbot` modify labels: rollup
rust-timer
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Apr 12, 2024
Rollup merge of rust-lang#122882 - Zoxc:panic-output-panic, r=Amanieu Avoid a panic in `set_output_capture` in the default panic handler This avoid a panic in the default panic handler by not using `set_output_capture` as `OUTPUT_CAPTURE.with` may panic once `OUTPUT_CAPTURE` is dropped. A new non-panicking `try_set_output_capture` variant of `set_output_capture` is added for use in the default panic handler.
|
This is the best, much-needed, PR! Thanks mucho! ignore this noise:it got me from this: to this: A definite win/progress and I didn't have to change anything :) For completion, that's running: export RUST_BACKTRACE=1
cargo clean
cargo test -- --test-threads=4 --format prettyin checked out rust-lang/cc-rs@bbae474 but with that line that the commit added, commented out, so it would fail the test. with this rustc work-in-progress personal Index: /var/tmp/portage/dev-lang/rust-1.76.0-r1/work/rustc-1.76.0-src/library/core/src/fmt/mod.rs
===================================================================
--- .orig/var/tmp/portage/dev-lang/rust-1.76.0-r1/work/rustc-1.76.0-src/library/core/src/fmt/mod.rs
+++ /var/tmp/portage/dev-lang/rust-1.76.0-r1/work/rustc-1.76.0-src/library/core/src/fmt/mod.rs
@@ -352,6 +352,20 @@ impl<'a> Arguments<'a> {
Arguments { pieces, fmt: Some(fmt), args }
}
+ /// just return the unformatted string
+ /// so this:
+ /// "some formatted string i={} j={}"
+ /// would be seen as:
+ /// "some formatted string i= j="
+ /// because the {}s get removed and transformed into the args array.
+ /// this assumes `pub fn write` was already patched to handle the extra pieces,
+ /// else you only see the first piece like:
+ /// "some formatted string i="
+ #[inline]
+ pub fn unformat_it(&self) -> Arguments<'a> {
+ Arguments { pieces:self.pieces, fmt: None, args: &[] }
+ }
+
/// Estimates the length of the formatted text.
///
/// This is intended to be used for setting initial `String` capacity
@@ -434,8 +448,22 @@ impl<'a> Arguments<'a> {
#[stable(feature = "rust1", since = "1.0.0")]
impl Debug for Arguments<'_> {
- fn fmt(&self, fmt: &mut Formatter<'_>) -> Result {
- Display::fmt(self, fmt)
+ fn fmt(&self, f: &mut Formatter<'_>) -> Result {
+ Display::fmt(self, f)
+// let fmt_display = match &self.fmt {
+// Some(fmt) => Display::fmt(fmt,f),//format!("{}", fmt),
+// None => "<no fmt>".to_string(),
+// };
+// let args_display = match &self.args {
+// Some(args) => format!("{}", args),
+// None => String::from("<no args>"),
+// };
+// f.debug_struct("Arguments")
+// .field("pieces", &self.pieces)
+// //.field("fmt", &self.fmt)
+// .field("fmt", &fmt_display)
+// .field("args", &fmt_display)
+// .finish()
}
}
@@ -1140,10 +1168,23 @@ pub fn write(output: &mut dyn Write, arg
}
// There can be only one trailing string piece left.
- if let Some(piece) = args.pieces.get(idx) {
- formatter.buf.write_str(*piece)?;
+ //if let Some(piece) = args.pieces.get(idx) {
+ // formatter.buf.write_str(*piece)?;
+ //}
+ //Maybe there's a ton of pieces*, that had fmt: None and args: &[]
+ //and we wanna print them all, yes they'll be unformatted like due to .unformat_it() function
+ //from Arguments struct.
+ // * not just the one trailing string piece left.
+ if idx < args.pieces.len() {
+ // Iterate over the remaining string pieces starting from idx
+ for piece in &args.pieces[idx..] {
+ // Write each piece to the formatter buffer
+ formatter.buf.write_str(*piece)?;
+ }
}
+
+
Ok(())
}
Index: /var/tmp/portage/dev-lang/rust-1.76.0-r1/work/rustc-1.76.0-src/library/std/src/lib.rs
===================================================================
--- .orig/var/tmp/portage/dev-lang/rust-1.76.0-r1/work/rustc-1.76.0-src/library/std/src/lib.rs
+++ /var/tmp/portage/dev-lang/rust-1.76.0-r1/work/rustc-1.76.0-src/library/std/src/lib.rs
@@ -403,6 +403,7 @@
// tidy-alphabetical-end
//
#![default_lib_allocator]
+#![feature(fmt_internals)] // used in https://github.com/rust-lang/rust/pull/122984/files#diff-88e2a536317b831c2e958b9205fde12f5edaabefba963bdd3a7503bbdedf8da9R752
// Explicitly import the prelude. The compiler uses this same unstable attribute
// to import the prelude implicitly when building crates that depend on std.
Index: /var/tmp/portage/dev-lang/rust-1.76.0-r1/work/rustc-1.76.0-src/library/std/src/panicking.rs
===================================================================
--- .orig/var/tmp/portage/dev-lang/rust-1.76.0-r1/work/rustc-1.76.0-src/library/std/src/panicking.rs
+++ /var/tmp/portage/dev-lang/rust-1.76.0-r1/work/rustc-1.76.0-src/library/std/src/panicking.rs
@@ -350,10 +350,18 @@ pub mod panic_count {
PanicInHook,
}
+ #[cfg(not(any(target_os = "linux", target_os = "windows")))]
+ compile_error!("This thread_local!() code only works w/o allocations on Linux or Windows platforms! I mean, \n
+ it's kinda guaranteed*(should double check) to not allocate on heap\n
+ on Linux/Windows but not sure on other platforms. And we don't want\n
+ it to allocate anything so that the 'fork' case below is respected.\n
+ *Well actually looks like not even this is guaranteed, see:\n
+ https://github.com/rust-lang/rust/issues/122940#issuecomment-2016600046");
// Panic count for the current thread and whether a panic hook is currently
// being executed..
thread_local! {
static LOCAL_PANIC_COUNT: Cell<(usize, bool)> = const { Cell::new((0, false)) }
+ //static PANIC_MULTIPATHS: //TODO? i mean, it should work still(even if fork, in practice): https://github.com/rust-lang/rust/issues/122940#issuecomment-2016600046
}
// Sum of panic counts from all threads. The purpose of this is to have
@@ -382,6 +390,7 @@ pub mod panic_count {
// Stealing a bit is fine because it just amounts to assuming that each
// panicking thread consumes at least 2 bytes of address space.
static GLOBAL_PANIC_COUNT: AtomicUsize = AtomicUsize::new(0);
+ //doneFIXME: temp 'pub'
// Increases the global and local panic count, and returns whether an
// immediate abort is required.
@@ -726,6 +735,7 @@ pub const fn begin_panic<M: Any + Send>(
}
}
+
/// Central point for dispatching panics.
///
/// Executes the primary logic for a panic, including checking for recursive
@@ -738,33 +748,156 @@ fn rust_panic_with_hook(
can_unwind: bool,
force_no_backtrace: bool,
) -> ! {
+ rtprintpanic!("!!!!!!!!!!!!!!! PANIC STARTING !!!!!!!!!!!!!!\n");
let must_abort = panic_count::increase(true);
+ rtprintpanic!("!! must_abort={:?}\n", must_abort);
+ //rtprintpanic!("must_abort={:?} global_panic_count={}\n", must_abort,panic_count::GLOBAL_PANIC_COUNT.load(Ordering::SeqCst));
+ let is_fork=crate::rt::is_this_forked_process();
+ rtprintpanic!("!! is_fork={:?}\n", is_fork);
+ let must_abort=if is_fork {
+ Some(panic_count::MustAbort::AlwaysAbort)
+ } else {
+ must_abort
+ };
+ rtprintpanic!("!! must_abort={:?}\n", must_abort);
// Check if we need to abort immediately.
if let Some(must_abort) = must_abort {
match must_abort {
panic_count::MustAbort::PanicInHook => {
- // Don't try to print the message in this case
- // - perhaps that is causing the recursive panics.
- rtprintpanic!("thread panicked while processing panic. aborting.\n");
- }
- panic_count::MustAbort::AlwaysAbort => {
+ rtprintpanic!("!!! in panic_count::MustAbort::PanicInHook\n");
+
+ //this is formatted message in a Some() TODO: if let?
+ rtprintpanic!("!!!0of2 message={:?}\n",message);//FIXME: use this first, if it panics in it, skip it next time.
+ let message2=message.map(|m| m.unformat_it()); // call message.unformat_it() only if
+ // message is Some, not when None
+ //rtprintpanic!("!!!0of2 message2={:?}\n",message2);
+ let panic_info = PanicInfo::internal_constructor(
+ message2.as_ref(),
+ //message_str.as_ref(),
+ location,
+ can_unwind,
+ false,//okTODO: is this actually false here? or is it false only
+ //during must abort variant/branch below?! actually this isn't part of its Display
+ //impl so it doesn't matter its value here, therefore i must use 'false' as it's
+ //what it does anyway, but know that it is ignored: https://github.com/rust-lang/rust/blob/42198bf562b548015e3eae3926c175c4aabb3a7b/library/core/src/panic/panic_info.rs#L159-L178
+ );
+ //rtprintpanic!("thread panicked while processing panic. aborting. msg='{:?}' loc='{:?}'\n",message, location);
+// let panic_info = PanicInfo {
+// payload, // this should already get printed as string: https://github.com/rust-lang/rust/blob/af98101ed89dba94309c64f1fbf37c890d988f9f/library/core/src/panic/panic_info.rs#L168-L171 well this won't work unless i use the payload.get() like below which actually does formatting which isn't what I want
+// message:None, // ^ but only if message is None
+// location,
+// can_unwind,
+// force_no_backtrace,
+// };
+ //FIXME: I need thread local(that doesn't do allocations when accessing it) and
+ //which will let me progressively remove things from the output which might panic
+ //like the 'message'(+loc+backtr) at first, then just 'location'(+backtr), then just the backtrace dump
+ //but can't guarantee TLS won't alloc atll (which we'd need for the oom case which will panic and get here)
+ //XXX: this won't show msg for opt.expect("some string") either because is formatted eg. it kinda uses panic!("{}", "some string") internally; https://github.com/rust-lang/rust/pull/122984/files#r1538124227
+ rtprintpanic!("!!!1of2 {panic_info}\n!!! thread panicked while processing panic. aborting. msg was not shown at all if it was formatted(above)\n");//, loc='{:?}'\n", location); location's already shown via panic_info's Display impl as first line.
+
+ //src: https://github.com/rust-lang/rust/pull/122984/files
+ // Don't try to format the message in this case, perhaps that is causing the
+ // recursive panics. However if the message is just a string, no user-defined
+ // code is involved in printing it, so that is risk-free.
+ // XXX: so this shows the message if it has no formatting, else it doesn't show it
+ // at all - by design: https://github.com/rust-lang/rust/pull/122984/files#r1536775570
+ // Not gonna lie, I didn't expect this code would just not show it at all, if it's
+ // formatted.
+ //rtprintpanic!("!!!0of2 message={:?}\n",message);//doneFIXME: remove these
+ let msg_str = message.and_then(|m| m.as_str()).map(|m| [m]);
+ let message1 = msg_str.as_ref().map(|m| fmt::Arguments::new_const(m));
+ //let message_str = match message {
+ // Some(args) => args.pieces.get(0).copied().map(fmt::Arguments::new_const),
+ // None => None,
+ //};
+ //rtprintpanic!("!!!0of2 message1={:?}\n",message1);
+ let panic_info = PanicInfo::internal_constructor(
+ message1.as_ref(),
+ //message_str.as_ref(),
+ location,
+ can_unwind,
+ false,//okTODO: is this actually false here? or is it false only
+ //during must abort variant/branch below?! actually this isn't part of its Display
+ //impl so it doesn't matter its value here, therefore i must use 'false' as it's
+ //what it does anyway, but know that it is ignored: https://github.com/rust-lang/rust/blob/42198bf562b548015e3eae3926c175c4aabb3a7b/library/core/src/panic/panic_info.rs#L159-L178
+ );
+ rtprintpanic!("!!!2of2 {panic_info}\n!!! thread panicked while processing panic. aborting. msg was not shown at all if it was formatted(above)\n");//, loc='{:?}'\n", location); location's already shown via panic_info's Display impl as first line.
+ //let panicinfo = PanicInfo::internal_constructor(
+ // message,//this would infinite panic
+ // location,
+ // can_unwind,
+ // force_no_backtrace,
+ //);
+ //rtprintpanic!("{panicinfo}\nhere's a stacktrace attempt:\n");
+ //FIXME: this would alloc here which is bad in oom or fork case(but fork case
+ //doesn't hit this currently, well unless u forgot to call panic::always_abort()
+ //in the fork and a double panic occurs)
+ rtprintpanic!("!!! FIXME: not printing a backtrace here, atm, to avoid alloc or infinite panics due to alloc might've been overriden in global allocator and set to panic!\n");
+ //rtprintpanic!("!! Forcing a backtrace (FIXME: allocs and could thus panic again for infinite panics):\n{}\n", crate::backtrace::Backtrace::force_capture());
+ }
+ panic_count::MustAbort::AlwaysAbort => { // this is hit on first and any subseq.
+ // (ie. recursive) panics(while always_abort() has been
+ // called before once(since it can't be
+ // reset/unset after)
+ rtprintpanic!("!!! in panic_count::MustAbort::AlwaysAbort\n");
// Unfortunately, this does not print a backtrace, because creating
- // a `Backtrace` will allocate, which we must to avoid here.
- let panicinfo = PanicInfo::internal_constructor(
- message,
+ // a `Backtrace` will allocate, which we must avoid here.
+// static BEEN_HERE:AtomicBool=AtomicBool::new(false);
+ //FIXME: this is the wrong way; need a thread local var, but one that doesn't alloc (for the fork case)
+ //actually can't use thread locals in fork either because it allocs (even the
+ //__thread thing in C could) so TODO: detect if fork then don't alloc or use TLS and
+ //just print simple+abort.
+// if false==BEEN_HERE.swap(true, Ordering::SeqCst) {
+ //let panicinfo = PanicInfo::internal_constructor(
+ // message,//FIXME: user controlled via Display impl(thus could alloc or panic again there)
+ // location,
+ // can_unwind,
+ // force_no_backtrace,
+ //);
+ //old way ^ was recursing because formatted msg could re-panic in Display impl of
+ //the passed args(to panic).
+ //src: https://github.com/rust-lang/rust/pull/122984/files
+ let msg_str = message.and_then(|m| m.as_str()).map(|m| [m]);
+ let message = msg_str.as_ref().map(|m| fmt::Arguments::new_const(m));
+ let panic_info = PanicInfo::internal_constructor(
+ message.as_ref(),//shows message if it has no formatting, else doesn't show it at all.
location,
can_unwind,
- force_no_backtrace,
+ false,//mehTODO: how do we know this 'force_no_backtrace' was set to false already? it's
+ //ignored in Display impl anyway! so setting to 'false' to be obvious.
);
- rtprintpanic!("{panicinfo}\npanicked after panic::always_abort(), aborting.\n");
+ rtprintpanic!("!!! {panic_info}\n!!! panicked after panic::always_abort(), aborting.\n");
+ //XXX: we'd restore this even if thread local because this whole thing may end up in a
+ //landing pad and some future new panic(like if this is ran within libtest via
+ //'cargo test' with --test-threads 1, it could be catch_unwind-ed aka
+ //caught, then the next test that panics gets back here)
+ //might get itself here and have an unclean path state from this old panic we're doing now.
+ //even the abort below might get caught by atexit hook(iirc) and end up in landing
+ //pad (well, it would with my hacky patch that's about that, iirc)
+ //Restoration of other paths would be done before the abort or return from this
+ //func. For the reasons above, ie. in case panic/abort is caught(eg. landing
+ //padded)
+// BEEN_HERE.store(false, Ordering::SeqCst);
+// } else {//been here, prevent recursion
+ //we assume something could've panic-ed above, but in truth only when message
+ //formatting woulda been shown it could panic in Display impl(user-code)
+ //but now that we're never showing formatted message...
+ //or also if some alloc happened(but it can't really, not in the above protected-section)
+// rtprintpanic!("!! panicked after panic::always_abort(), aborting. (and recursion prevented) loc='{:?}'\n", location);
+// }
}
}
+ rtprintpanic!("!!! about to abort_internal() which does a libc::abort() in fact.\n");
+ rtprintpanic!(concat!(r"\\\\\\\\\\\\\\ PANIC ENDING (1) //////////","\n"));
crate::sys::abort_internal();
}
+ rtprintpanic!("!! about to mut info\n");
let mut info =
PanicInfo::internal_constructor(message, location, can_unwind, force_no_backtrace);
+ rtprintpanic!("!! about to hook read\n");
let hook = HOOK.read().unwrap_or_else(PoisonError::into_inner);
match *hook {
// Some platforms (like wasm) know that printing to stderr won't ever actually
@@ -773,31 +906,50 @@ fn rust_panic_with_hook(
// methods, this means we avoid formatting the string at all!
// (The panic runtime might still call `payload.take_box()` though and trigger
// formatting.)
- Hook::Default if panic_output().is_none() => {}
+ Hook::Default if panic_output().is_none() => {
+ rtprintpanic!("!!! 1\n");
+ }
Hook::Default => {
- info.set_payload(payload.get());
+ rtprintpanic!("!!! 2\n");
+ let p=payload.get();
+ rtprintpanic!("!!! 22\n");
+ info.set_payload(p);
+ rtprintpanic!("!!! 222\n");
default_hook(&info);
}
Hook::Custom(ref hook) => {
- info.set_payload(payload.get());
+ rtprintpanic!("!!! 3\n");
+ let p=payload.get();
+ rtprintpanic!("!!! 33\n");
+ info.set_payload(p);
+ //info.set_payload(payload.get());
+ rtprintpanic!("!!! 333\n");
hook(&info);
}
};
+ rtprintpanic!("!! 4\n");
drop(hook);
+ rtprintpanic!("!! 5\n");
// Indicate that we have finished executing the panic hook. After this point
// it is fine if there is a panic while executing destructors, as long as it
// it contained within a `catch_unwind`.
panic_count::finished_panic_hook();
+ rtprintpanic!("!! 6\n");
if !can_unwind {
+ rtprintpanic!("!!! 7\n");
// If a thread panics while running destructors or tries to unwind
// through a nounwind function (e.g. extern "C") then we cannot continue
// unwinding and have to abort immediately.
- rtprintpanic!("thread caused non-unwinding panic. aborting.\n");
+ rtprintpanic!("!!! thread caused non-unwinding panic. aborting. loc={:?}\n", location);
+ rtprintpanic!("!!! 8ai\n");
+ rtprintpanic!(concat!(r"\\\\\\\\\\\\\\ PANIC ENDING (2) //////////","\n"));
crate::sys::abort_internal();
}
+ rtprintpanic!("!! 9rp\n");
+ rtprintpanic!(concat!(r"\\\\\\\\\\\\\\ PANIC ENDING (3) //////////","\n"));
rust_panic(payload)
}
Index: /var/tmp/portage/dev-lang/rust-1.76.0-r1/work/rustc-1.76.0-src/library/std/src/rt.rs
===================================================================
--- .orig/var/tmp/portage/dev-lang/rust-1.76.0-r1/work/rustc-1.76.0-src/library/std/src/rt.rs
+++ /var/tmp/portage/dev-lang/rust-1.76.0-r1/work/rustc-1.76.0-src/library/std/src/rt.rs
@@ -68,6 +68,65 @@ macro_rules! rtunwrap {
};
}
+pub mod isolated_globals {
+ //Initially it's not forked, only after fork() is called.
+ //TODO: do I even need atomic? are writes not sync-ed? presumably the forked process is only 1
+ //thread anyway even before the value is set to 'true', then any new threads(if any are even
+ //allowed), wouldn't need sync-ed read to this bool, would they? visibility-wise.
+ //pub(super) static mut IS_THIS_FORKED_PROCESS:bool=false;
+ use core::sync::atomic::AtomicBool;
+ pub(super) static IS_THIS_FORKED_PROCESS_AB:AtomicBool=AtomicBool::new(false);
+
+// // TODO: delete this
+// #[inline]
+// #[track_caller]
+// pub fn get_current_process_id() -> u32 {
+// crate::process::id()
+// }
+
+
+ // None means we can't determine.
+ #[track_caller]
+ #[inline(always)]
+ pub fn is_this_main_process() -> bool {
+ return !super::is_this_forked_process();
+ }
+
+ #[cfg(any(unix, target_os = "fuchsia", target_os = "vxworks"))]
+ pub(super) fn init_fork_hooks() {
+ unsafe {
+ let result: libc::c_int = libc::pthread_atfork(/*prepare*/None, /*parent*/None, Some(child));
+ if result != 0 {
+ //"On success, pthread_atfork() returns zero. On error, it returns an error number." - man
+ //ERRORS
+ // ENOMEM Could not allocate memory to record the fork handler list entry.
+ //TODO: more details
+ eprintln!("!!! libc::pthread_atfork() failed with error code: '{}'. Forks won't be detected!", result);
+ return;
+ }
+ }//unsafe
+ }//fn
+
+ /// this executes in forked process before returning control from fork() call.
+ //"child specifies a handler that is executed in the child process after fork(2) processing completes." - man
+ #[cfg(any(unix, target_os = "fuchsia", target_os = "vxworks"))]
+ unsafe extern "C" fn child() {
+ //unsafe { // it's always set to same 'true', and it's done in a new process
+ // IS_THIS_FORKED_PROCESS=true;
+ //}
+ IS_THIS_FORKED_PROCESS_AB.store(true, core::sync::atomic::Ordering::Release);
+ }
+}//mod
+
+/// True if this process, whether it's a direct or indirect descendant of main, was forked.
+/// it can be a fork of a forked process, it's still detected as a fork.
+#[track_caller]
+#[inline(always)]
+pub fn is_this_forked_process() -> bool {
+ //return unsafe { isolated_globals::IS_THIS_FORKED_PROCESS };
+ return isolated_globals::IS_THIS_FORKED_PROCESS_AB.load(core::sync::atomic::Ordering::Acquire);
+}
+
// One-time runtime initialization.
// Runs before `main`.
// SAFETY: must be called only once during runtime initialization.
@@ -94,7 +153,10 @@ macro_rules! rtunwrap {
#[cfg_attr(test, allow(dead_code))]
unsafe fn init(argc: isize, argv: *const *const u8, sigpipe: u8) {
unsafe {
- sys::init(argc, argv, sigpipe);
+ sys::init(argc, argv, sigpipe); //TODO: find out where and what this does!
+
+ #[cfg(any(unix, target_os = "fuchsia", target_os = "vxworks"))]
+ isolated_globals::init_fork_hooks();
let main_guard = sys::thread::guard::init();
// Next, set up the current Thread with the guard information we just
Index: /var/tmp/portage/dev-lang/rust-1.76.0-r1/work/rustc-1.76.0-src/library/test/src/formatters/pretty.rs
===================================================================
--- .orig/var/tmp/portage/dev-lang/rust-1.76.0-r1/work/rustc-1.76.0-src/library/test/src/formatters/pretty.rs
+++ /var/tmp/portage/dev-lang/rust-1.76.0-r1/work/rustc-1.76.0-src/library/test/src/formatters/pretty.rs
@@ -91,7 +91,10 @@ impl<T: Write> PrettyFormatter<T> {
pub fn write_plain<S: AsRef<str>>(&mut self, s: S) -> io::Result<()> {
let s = s.as_ref();
self.out.write_all(s.as_bytes())?;
- self.out.flush()
+ let res=self.out.flush();
+ use std::io::stdout;
+ stdout().flush()?;
+ return res;
}
fn write_time(
@@ -173,6 +176,9 @@ impl<T: Write> PrettyFormatter<T> {
if let Some(test_mode) = desc.test_mode() {
self.write_plain(format!("test {name} - {test_mode} ... "))?;
} else {
+ for i in 1..=5 {
+ self.write_plain(format!("test {name} ... {i}\n"))?;
+ }
self.write_plain(format!("test {name} ... "))?;
}
Index: /var/tmp/portage/dev-lang/rust-1.76.0-r1/work/rustc-1.76.0-src/library/test/src/lib.rs
===================================================================
--- .orig/var/tmp/portage/dev-lang/rust-1.76.0-r1/work/rustc-1.76.0-src/library/test/src/lib.rs
+++ /var/tmp/portage/dev-lang/rust-1.76.0-r1/work/rustc-1.76.0-src/library/test/src/lib.rs
@@ -91,6 +91,22 @@ use options::RunStrategy;
use test_result::*;
use time::TestExecTime;
+/// Executes block based on the value of environment variable TEADEBUG
+/// eg. $ TEADEBUG=4 cargo run
+/// set to 0 or unset, to not execute block;
+/// set to 1 or anything non-number to execute blocks with level 1
+/// set to any number >1 to execute blocks with that level or below it!
+macro_rules! tea {
+ ($level:expr, $block:block) => {
+ if let Ok(var_value) = std::env::var("TEADEBUG") {
+ let level= var_value.parse::<u32>().unwrap_or(1);
+ if level > 0 && $level <= level {
+ $block
+ }
+ }
+ };
+}
+
// Process exit code to be used to indicate test failures.
const ERROR_EXIT_CODE: i32 = 101;
@@ -379,6 +395,9 @@ where
}
if concurrency == 1 {
+ tea!(1,{
+ println!("!!!!!!!!!!! concurrency==1");
+ });
while !remaining.is_empty() {
let (id, test) = remaining.pop_front().unwrap();
let event = TestEvent::TeWait(test.desc.clone());
@@ -401,11 +420,32 @@ where
}
}
} else {
+ tea!(1,{
+ println!("!!!!!!!!!!! concurrency=={}",concurrency);
+ });
while pending > 0 || !remaining.is_empty() {
+ /*let mut which_test:TestDesc=TestDesc { name:types::TestName::StaticTestName("not yet"),
+
+ ignore: true,
+ ignore_message: None,
+ source_file: "meh",
+ start_line: 0,
+ start_col: 0,
+ end_line: 0,
+ end_col: 0,
+ should_panic: options::ShouldPanic::Yes,
+ compile_fail: true,
+ no_run: true,
+ test_type: TestType::Unknown,
+ };*/
while pending < concurrency && !remaining.is_empty() {
let (id, test) = remaining.pop_front().unwrap();
+ tea!(5,{
+ println!("!!!!!!!!!!! first while for test={:?}",test.desc.name);
+ });
let timeout = time::get_default_test_timeout();
let desc = test.desc.clone();
+ //which_test=desc.clone();
let event = TestEvent::TeWait(desc.clone());
notify_about_test_event(event)?; //here no pad
@@ -421,7 +461,10 @@ where
if let Some(timeout) = calc_timeout(&timeout_queue) {
res = rx.recv_timeout(timeout);
for test in get_timed_out_tests(&running_tests, &mut timeout_queue) {
- let event = TestEvent::TeTimeout(test);
+ let event = TestEvent::TeTimeout(test.clone());
+ tea!(10,{
+ println!("!!!!!!! Got event={:?} for test={:?}", &event, &test);
+ });
notify_about_test_event(event)?;
}
@@ -436,6 +479,9 @@ where
}
} else {
res = rx.recv().map_err(|_| RecvTimeoutError::Disconnected);
+ tea!(10,{
+ println!("!!!!!!! Got disconnect");
+ });
break;
}
}
@@ -443,23 +489,36 @@ where
let mut completed_test = res.unwrap();
let running_test = running_tests.remove(&completed_test.id).unwrap();
running_test.join(&mut completed_test);
+ tea!(4,{
+ println!("!!!!!!!!!!! completed_test={:?}", &completed_test.desc.name);
+ });
let fail_fast = match completed_test.result {
TrIgnored | TrOk | TrBench(_) => false,
TrFailed | TrFailedMsg(_) | TrTimedFail => opts.fail_fast,
};
- let event = TestEvent::TeResult(completed_test);
+ let event = TestEvent::TeResult(completed_test.clone());
notify_about_test_event(event)?;
pending -= 1;
if fail_fast {
// Prevent remaining test threads from panicking
+ tea!(10, {
+ println!("!!!!!!!!!!!1 Failing fast");
+ });
std::mem::forget(rx);
return Ok(());
- }
- }
- }
+ } else {
+ tea!(10, {
+ println!("!!!!!!!!!!! NOT Failing fast for test={:?}",&completed_test.desc.name);
+ });
+ }
+ } //while
+ tea!(1,{
+ println!("!!!!!!!!!!! while is done with pending={} and remaining.is_empty()={}",pending , remaining.is_empty());
+ });
+ } //else concurrent!
if opts.bench_benchmarks {
// All benchmarks run at the end, in serial.
@@ -558,11 +617,18 @@ pub fn run_test(
match testfn.into_runnable() {
Runnable::Test(runnable_test) => {
if runnable_test.is_dynamic() {
+ tea!(10,{
+ println!("!!!!!! is dynamic for test={:?}",&desc.name);
+ });
match strategy {
RunStrategy::InProcess => (),
_ => panic!("Cannot run dynamic test fn out-of-process"),
};
- }
+ } else {
+ tea!(10,{
+ println!("!!!!!! is NOT dynamic for test={:?}",&desc.name);
+ });
+ }//if
let name = desc.name.clone();
let nocapture = opts.nocapture;
@@ -595,6 +661,9 @@ pub fn run_test(
// level.
let supports_threads = !cfg!(target_os = "emscripten") && !cfg!(target_family = "wasm");
if supports_threads {
+ tea!(7,{
+ println!("!!!!!!!!! spawning test thread for {:?}",&name);
+ });
let cfg = thread::Builder::new().name(name.as_slice().to_owned());
let mut runtest = Arc::new(Mutex::new(Some(runtest)));
let runtest2 = runtest.clone();
@@ -630,6 +699,7 @@ fn __rust_begin_short_backtrace<T, F: Fn
black_box(result)
}
+
fn run_test_in_process(
id: TestId,
desc: TestDesc,
@@ -639,29 +709,230 @@ fn run_test_in_process(
monitor_ch: Sender<CompletedTest>,
time_opts: Option<time::TestTimeOptions>,
) {
+ let tid=thread::current().id();
+ tea!(5,{
+ println!("!!!!! Running test in process(tid={:?}) test={:?} testid={:?} pid={}", tid, &desc.name, id, std::process::id());
+ });
// Buffer for capturing standard I/O
let data = Arc::new(Mutex::new(Vec::new()));
+// use std::io::stdout;
+// let _=stdout().flush();
+ //use std::io::stdout;
+// let _=stdout().flush();
+ //println!("!!!!!!! way before calling .run() for test={:?} pid={}", &desc.name, std::process::id());
+
+ //use libc::atexit;//TODO: ideally use libc which is in ../../vendor/libc/Cargo.toml (from libtest's project root) - don't know how yet without needing Cargo.lock modification which is forbidden from this gentoo .ebuild due to --locked
+
+ // Declare the external C function atexit
+ //use std::os::raw::c_int;
+ // Thread-local flag to determine whether cleanup should be performed
+ use std::cell::RefCell;
+ //thread_local! {
+ // static SHOULD_CLEANUP: RefCell<bool> = RefCell::new(true);
+ //}
+ // Atomic flag to determine whether cleanup should be performed
+ //use std::sync::atomic::AtomicBool;//,Ordering};
+ use std::sync::atomic::{AtomicU64,Ordering};
+ static REGISTERED_HOOKS_SO_FAR: AtomicU64 = AtomicU64::new(0);
+ //presumably you can't/shouldn't register more than 32 (man 3 atexit) and there may
+ //already be user-set ones TODO: test for this, see how this behaves!
+
+ static SHOULD_CLEANUP: AtomicU64 = AtomicU64::new(0);
+ extern "C" {
+ pub fn atexit(callback: extern "C" fn()) -> std::os::raw::c_int;
+ }
+ #[allow(dead_code)]
+ extern "C" fn cleanup() {
+ //once you hit this point it means u're in atexit hook so it won't run again on
+ //panic!+unwind_catch, thus need to "say" it's deregistered as hook:
+ REGISTERED_HOOKS_SO_FAR.fetch_sub(1, Ordering::SeqCst);
+
+ // Check if any threads were still active when this std::process:exit(?) happened.
+ let active_threads = SHOULD_CLEANUP.load(Ordering::SeqCst);
+ if active_threads > 0 {
+ tea!(5,{
+ println!("!!!! atexit HOOK hit! pid={}",std::process::id());
+ });
+ //io::set_output_capture(None);//FIXME: this can't exec after we've paniced(when we return
+ //from run() below, else it will double panic); double
+ //panics still even here!
+ //this next panic + catch_unwind below acts like an unregistering of this atexit hook.
+ panic!("!!!! Test has issued a std::process::exit(?), we're panicking to allow the test harness to catch this and properly report the test results. If you used --test-threads=1 you can still see which test caused this even without this workaround implemented!");
+ } else {
+ tea!(2,{
+ println!("!!!! nothing to clean up, test(s) behaved. hooks so far={}", REGISTERED_HOOKS_SO_FAR.load(Ordering::SeqCst));
+ });
+ }
+ }
+
+ SHOULD_CLEANUP.fetch_add(1, Ordering::SeqCst);//XXX: must be +1 before the 'if' below!
+
+ //FIXME: this check then add should be atomic, maybe use .with() ? what about the other
+ //places?!
+ let hooks_so_far=REGISTERED_HOOKS_SO_FAR.load(Ordering::SeqCst);
+ let active_test_threads_currently=SHOULD_CLEANUP.load(Ordering::SeqCst);
+ if hooks_so_far < active_test_threads_currently {
+// if ! DID_REGISTER.load(Ordering::SeqCst) {
+ //XXX: each concurrent running test thread needs one atexit hook, because we're assuming worst
+ //case scenario that all currently running test threads will exit() so they'll each enter one
+ //hook and then the panic!+catch_unwind will deregister the hook so to speak and the test
+ //thread will continue to live and report as failed.
+ //FIXME: problem is, we're not supposed to have more than 32 tops (man 3 atexit) and that's not
+ //counting user-set ones! so this is very hacky at best!
+ //XXX: should only register X max atexit hooks, where X is the max number of concurrent
+ //possible tests, eg. maybe from --test-threads=X arg. to the test harness!
+ //All these registered hooks cumulate and are run at main process exit, or when any test thread tries to
+ //abort() or exit(), but not when they just end normally!
+ // Register the exit handler
+ unsafe {
+ let result = atexit(cleanup);
+ if result != 0 {
+ //FIXME: test how this panic behaves here.
+ panic!("Failed to register exit handler(for test='{:?}') which would handle the case when a test used std::process::exit(?) instead of panic!()", &desc.name);
+ } else {
+ //DID_REGISTER.store(true, Ordering::SeqCst);
+ tea!(3,{
+ println!("!!!!! REGISTERed atexit handler, for test={:?} tid={:?}", &desc.name, tid);
+ });
+ REGISTERED_HOOKS_SO_FAR.fetch_add(1, Ordering::SeqCst);
+ }
+ }
+ } else {
+ tea!(3,{
+ println!("!!!!! ALREADY registered {hooks_so_far}/{active_test_threads_currently} atexit handlers, for test={:?} tid={:?}, not registering a new one.", &desc.name, tid);
+ });
+ }
+
+ //println!("!!!! BEFORE RUN, should={}", SHOULD_CLEANUP.with(|flag| *flag.borrow()));
+ tea!(4,{
+ println!("!!!! BEFORE RUN, should={} tid={:?}", SHOULD_CLEANUP.load(Ordering::SeqCst),tid);
+ });
if !nocapture {
io::set_output_capture(Some(data.clone()));
+ //} else {
+ //io::set_output_capture(None);
}
+ if "0" != std::env::var("TEAPANICHOOK").unwrap_or("0".to_string()) {
+ //let old_hook = panic::take_hook();
+ // Install a custom panic handler XXX: apparently it's needed else "thread panicked while processing panic. aborting." while doing the cc-rs 'cargo test' with clang_android test failing(unshimmed llvm-ar PR 1016)
+ // ok this is needed because of double panic without "--no-capture" arg, in the panic hook at line 147 in this file,
+ // where it calls io::set_output_capture(None) results in:
+ // "cannot access a Thread Local Storage value during or after destruction: AccessError"
+ panic::set_hook(Box::new(move |panic_info| { //nvmXXX: this was only needed to temporarily find a
+ //double panic caused by io::set_output_capture(None);
+ // Track whether panic handler has been called once
+ //let panic_handler_called_once = AtomicBool::new(false);
+ thread_local! {
+ static CALLED_ONCE: RefCell<bool> = RefCell::new(false);
+ }
+ let double_panic=
+ // Check if CALLED_ONCE is true, if yes, abort, otherwise set it to true
+ CALLED_ONCE.with(|called_once| {
+ let mut called_once = called_once.borrow_mut();
+ if *called_once {
+ true
+ } else {
+ *called_once = true;
+ false
+ }
+ });
+
+ let double_text=
+ if double_panic {
+ "double "
+ } else {
+ ""
+ };
+ // Print the panic message
+ if let Some(message) = panic_info.payload().downcast_ref::<&str>() {
+ println!("Custom panic handler caught {}panic: {}", double_text,message);
+ } else {
+ println!("Custom panic handler caught {}panic",double_text);
+ }
+
+ // Print a backtrace if available
+ if let Some(location) = panic_info.location() {
+ println!("Panic occurred in file '{}' at line {}", location.file(), location.line());
+ println!("{}", std::backtrace::Backtrace::capture());
+ }
+
+ if double_panic {
+ println!("Aborting the process due to double panic detected...");
+ println!("{}", std::backtrace::Backtrace::capture());
+ // Deregister the current panic hook to let the normal panic hook take over
+ //panic::set_hook(old_hook);
+ //process::abort();
+ //let _ = panic::take_hook(); //Unregisters the current panic hook and returns it, registering the default hook in its place. If the default hook is registered it will be returned, but remain registered.
+ //panic!("{}", panic_info);
+ //let def_hook = panic::take_hook();
+ //def_hook(panic_info);
+ }
+ //// Check if panic handler has been called once before
+ ////if panic_handler_called_once.load(Ordering::Relaxed) {
+
+ // // This is a double panic, abort the process
+ // process::abort();
+ //} else {
+ // // This is the first panic, mark panic handler as called
+ // //panic_handler_called_once.store(true, Ordering::Relaxed);
+ // // Set CALLED_ONCE to true
+ // CALLED_ONCE.with(|called_once| {
+ // *called_once.borrow_mut() = true;
+ // });
+ //}
+ }));
+ }//if TEAPANICHOOK
+
let start = report_time.then(Instant::now);
let result = fold_err(catch_unwind(AssertUnwindSafe(|| runnable_test.run())));
+ //SHOULD_CLEANUP.with(|flag| *flag.borrow_mut() = false);//test is done, don't panic anymore after this
+ //SHOULD_CLEANUP.store(false, Ordering::Relaxed);
+ SHOULD_CLEANUP.fetch_sub(1, Ordering::SeqCst);
let exec_time = start.map(|start| {
let duration = start.elapsed();
TestExecTime(duration)
});
-
- io::set_output_capture(None);
-
- let test_result = match result {
- Ok(()) => calc_result(&desc, Ok(()), &time_opts, &exec_time),
- Err(e) => calc_result(&desc, Err(e.as_ref()), &time_opts, &exec_time),
- };
- let stdout = data.lock().unwrap_or_else(|e| e.into_inner()).to_vec();
- let message = CompletedTest::new(id, desc, test_result, exec_time, stdout);
- monitor_ch.send(message).unwrap();
+ //io::set_output_capture(None);//this causes double panic hmmm, first being in atexit hook!
+ //println!("!!!!!!! returned from .run() of test={:?}", &desc.name);
+ //println!("!!!! AFTER RUN, should={}", SHOULD_CLEANUP.with(|flag| *flag.borrow()));
+ //let result = fold_err(catch_unwind(|| runnable_test.run()));
+ //cleanup();
+
+
+ let test_result = match result {
+ Ok(()) => {
+ io::set_output_capture(None);//this panics(ie. double) if not here in Ok()
+
+ //FIXME: dedup, but must be after io::set_output_capture(None);
+ tea!(4,{
+ println!("!!!! AFTER RUN(good), should={} tid={:?} test={:?}", SHOULD_CLEANUP.load(Ordering::SeqCst),tid, &desc.name);
+ });
+
+ let tid_after=thread::current().id();//this panics if it was a test that used
+ //abort() or process::exit()
+ if tid != tid_after {
+ println!("!!!! thread ID before/after run differ {:?}!={:?}",tid, tid_after);
+ }
+ assert_eq!(tid, tid_after);
+ //hmm can't really know, tid would be same if forked.
+ calc_result(&desc, Ok(()), &time_opts, &exec_time)
+ },
+ Err(e) => {
+ //FIXME: dedup, but must be after io::set_output_capture(None);
+ tea!(4,{
+ println!("!!!! AFTER RUN(err), should={} tid={:?} test={:?}", SHOULD_CLEANUP.load(Ordering::SeqCst),tid, &desc.name);
+ });
+ calc_result(&desc, Err(e.as_ref()), &time_opts, &exec_time)
+ },
+ };
+ let stdout = data.lock().unwrap_or_else(|e| e.into_inner()).to_vec();
+ let message = CompletedTest::new(id, desc, test_result, exec_time, stdout);
+ tea!(11,{
+ println!("!!!!!!! message={:?}", String::from_utf8_lossy(&message.stdout));
+ });
+ monitor_ch.send(message).unwrap();
}
fn fold_err<T, E>(
@@ -686,6 +957,10 @@ fn spawn_test_subprocess(
time_opts: Option<time::TestTimeOptions>,
bench_benchmarks: bool,
) {
+ //tea!(5,{
+ //since this isn't reached in my so-far tests, i wanna see it always:
+ println!("!!!!! Running test in SUBprocess test={:?}", &desc);
+ //});
let (result, test_output, exec_time) = (|| {
let args = env::args().collect::<Vec<_>>();
let current_exe = &args[0];
@@ -755,9 +1030,12 @@ fn run_test_in_spawned_subprocess(desc:
}
if let TrOk = test_result {
+ println!("!!!!!!!!!! OK test={:?}", desc);
process::exit(test_result::TR_OK);
} else {
- process::exit(test_result::TR_FAILED);
+ println!("!!!!!!!!!! boom test failed test={:?}", desc);
+ process::abort();
+ //process::exit(test_result::TR_FAILED);
}
});
let record_result2 = record_result.clone(); |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This avoid a panic in the default panic handler by not using
set_output_captureasOUTPUT_CAPTURE.withmay panic onceOUTPUT_CAPTUREis dropped.A new non-panicking
try_set_output_capturevariant ofset_output_captureis added for use in the default panic handler.