auto merge of #11353 : alexcrichton/rust/improve-logging, r=brson

This will allow capturing of common things like logging messages, stdout prints
(using stdio println), and failure messages (printed to stderr).  Any new prints
added to libstd should be funneled through these task handles to allow capture
as well.

Additionally, this commit redirects logging back through a `Logger` trait so the
log level can be usefully consumed by an arbitrary logger.

This commit also introduces methods to set the task-local stdout handles:

* std::io::stdio::set_stdout
* std::io::stdio::set_stderr
* std::io::logging::set_logger

These methods all return the previous logger just in case it needs to be used
for inspection.

I plan on using this infrastructure for extra::test soon, but we don't quite
have the primitives that I'd like to use for it, so it doesn't migrate
extra::test at this time.

Closes #6369

3 files changed, 102 insertions, 88 deletions

diff --git a/src/libstd/rt/logging.rs b/src/libstd/rt/logging.rs
index dfcf49734df..586d26a24e3 100644
--- a/src/libstd/rt/logging.rs
+++ b/src/libstd/rt/logging.rs
@@ -8,13 +8,9 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-use fmt;
 use from_str::from_str;
 use libc::exit;
 use option::{Some, None, Option};
-use io;
-use io::stdio::StdWriter;
-use io::buffered::LineBufferedWriter;
 use rt::crate_map::{ModEntry, CrateMap, iter_crate_map, get_crate_map};
 use str::StrSlice;
 use vec::{ImmutableVector, MutableTotalOrdVector};
@@ -168,28 +164,6 @@ fn update_log_settings(crate_map: &CrateMap, settings: ~str) {
     }
 }
 
-pub trait Logger {
-    fn log(&mut self, args: &fmt::Arguments);
-}
-
-/// This logger emits output to the stderr of the process, and contains a lazily
-/// initialized event-loop driven handle to the stream.
-pub struct StdErrLogger {
-    priv handle: LineBufferedWriter<StdWriter>,
-}
-
-impl StdErrLogger {
-    pub fn new() -> StdErrLogger {
-        StdErrLogger { handle: LineBufferedWriter::new(io::stderr()) }
-    }
-}
-
-impl Logger for StdErrLogger {
-    fn log(&mut self, args: &fmt::Arguments) {
-        fmt::writeln(&mut self.handle as &mut io::Writer, args);
-    }
-}
-
 /// Configure logging by traversing the crate map and setting the
 /// per-module global logging flags based on the logging spec
 pub fn init() {
diff --git a/src/libstd/rt/task.rs b/src/libstd/rt/task.rs
index 27f9f0dbfa3..2d9105d6766 100644
--- a/src/libstd/rt/task.rs
+++ b/src/libstd/rt/task.rs
@@ -20,6 +20,7 @@ use cleanup;
 use io::Writer;
 use iter::{Iterator, Take};
 use local_data;
+use logging::Logger;
 use ops::Drop;
 use option::{Option, Some, None};
 use prelude::drop;
@@ -29,7 +30,6 @@ use rt::borrowck::BorrowRecord;
 use rt::borrowck;
 use rt::local::Local;
 use rt::local_heap::LocalHeap;
-use rt::logging::StdErrLogger;
 use rt::rtio::LocalIo;
 use rt::unwind::Unwinder;
 use send_str::SendStr;
@@ -55,8 +55,9 @@ pub struct Task {
     // Dynamic borrowck debugging info
     borrow_list: Option<~[BorrowRecord]>,
 
-    logger: Option<StdErrLogger>,
-    stdout_handle: Option<~Writer>,
+    logger: Option<~Logger>,
+    stdout: Option<~Writer>,
+    stderr: Option<~Writer>,
 
     priv imp: Option<~Runtime>,
 }
@@ -94,7 +95,8 @@ impl Task {
             name: None,
             borrow_list: None,
             logger: None,
-            stdout_handle: None,
+            stdout: None,
+            stderr: None,
             imp: None,
         }
     }
@@ -123,13 +125,21 @@ impl Task {
 
             // Run the task main function, then do some cleanup.
             f.finally(|| {
-                fn flush(w: Option<~Writer>) {
-                    match w {
-                        Some(mut w) => { w.flush(); }
-                        None => {}
-                    }
+                fn close_outputs() {
+                    let mut task = Local::borrow(None::<Task>);
+                    let logger = task.get().logger.take();
+                    let stderr = task.get().stderr.take();
+                    let stdout = task.get().stdout.take();
+                    drop(task);
+                    drop(logger); // loggers are responsible for flushing
+                    match stdout { Some(mut w) => w.flush(), None => {} }
+                    match stderr { Some(mut w) => w.flush(), None => {} }
                 }
 
+                // First, flush/destroy the user stdout/logger because these
+                // destructors can run arbitrary code.
+                close_outputs();
+
                 // First, destroy task-local storage. This may run user dtors.
                 //
                 // FIXME #8302: Dear diary. I'm so tired and confused.
@@ -161,16 +171,12 @@ impl Task {
                 // Destroy remaining boxes. Also may run user dtors.
                 unsafe { cleanup::annihilate(); }
 
-                // Finally flush and destroy any output handles which the task
-                // owns. There are no boxes here, and no user destructors should
-                // run after this any more.
-                let mut task = Local::borrow(None::<Task>);
-                let stdout = task.get().stdout_handle.take();
-                let logger = task.get().logger.take();
-                drop(task);
-
-                flush(stdout);
-                drop(logger);
+                // Finally, just in case user dtors printed/logged during TLS
+                // cleanup and annihilation, re-destroy stdout and the logger.
+                // Note that these will have been initialized with a
+                // runtime-provided type which we have control over what the
+                // destructor does.
+                close_outputs();
             })
         };
 
diff --git a/src/libstd/rt/unwind.rs b/src/libstd/rt/unwind.rs
index 217782195e6..cb5360200d5 100644
--- a/src/libstd/rt/unwind.rs
+++ b/src/libstd/rt/unwind.rs
@@ -61,12 +61,14 @@ use cast;
 use kinds::Send;
 use libc::{c_void, c_char, size_t};
 use option::{Some, None, Option};
+use prelude::drop;
 use result::{Err, Ok};
 use rt::local::Local;
 use rt::task::Task;
 use str::Str;
 use task::TaskResult;
 use unstable::intrinsics;
+use util;
 
 use uw = self::libunwind;
 
@@ -385,58 +387,90 @@ pub fn begin_unwind_raw(msg: *c_char, file: *c_char, line: size_t) -> ! {
 
 /// This is the entry point of unwinding for fail!() and assert!().
 pub fn begin_unwind<M: Any + Send>(msg: M, file: &'static str, line: uint) -> ! {
-    unsafe {
-        let task: *mut Task;
-        // Note that this should be the only allocation performed in this block.
-        // Currently this means that fail!() on OOM will invoke this code path,
-        // but then again we're not really ready for failing on OOM anyway. If
-        // we do start doing this, then we should propagate this allocation to
-        // be performed in the parent of this task instead of the task that's
-        // failing.
-        let msg = ~msg as ~Any;
+    // Note that this should be the only allocation performed in this block.
+    // Currently this means that fail!() on OOM will invoke this code path,
+    // but then again we're not really ready for failing on OOM anyway. If
+    // we do start doing this, then we should propagate this allocation to
+    // be performed in the parent of this task instead of the task that's
+    // failing.
+    let msg = ~msg as ~Any;
+
+    let mut task;
+    {
+        let msg_s = match msg.as_ref::<&'static str>() {
+            Some(s) => *s,
+            None => match msg.as_ref::<~str>() {
+                Some(s) => s.as_slice(),
+                None => "~Any",
+            }
+        };
+
+        // It is assumed that all reasonable rust code will have a local task at
+        // all times. This means that this `try_take` will succeed almost all of
+        // the time. There are border cases, however, when the runtime has
+        // *almost* set up the local task, but hasn't quite gotten there yet. In
+        // order to get some better diagnostics, we print on failure and
+        // immediately abort the whole process if there is no local task
+        // available.
+        let opt_task: Option<~Task> = Local::try_take();
+        task = match opt_task {
+            Some(t) => t,
+            None => {
+                rterrln!("failed at '{}', {}:{}", msg_s, file, line);
+                unsafe { intrinsics::abort() }
+            }
+        };
 
+        // See comments in io::stdio::with_task_stdout as to why we have to be
+        // careful when using an arbitrary I/O handle from the task. We
+        // essentially need to dance to make sure when a task is in TLS when
+        // running user code.
+        let name = task.name.take();
         {
-            let msg_s = match msg.as_ref::<&'static str>() {
-                Some(s) => *s,
-                None => match msg.as_ref::<~str>() {
-                    Some(s) => s.as_slice(),
-                    None => "~Any",
+            let n = name.as_ref().map(|n| n.as_slice()).unwrap_or("<unnamed>");
+
+            match task.stderr.take() {
+                Some(mut stderr) => {
+                    Local::put(task);
+                    format_args!(|args| ::fmt::writeln(stderr, args),
+                                 "task '{}' failed at '{}', {}:{}",
+                                 n, msg_s, file, line);
+                    task = Local::take();
+
+                    match util::replace(&mut task.stderr, Some(stderr)) {
+                        Some(prev) => {
+                            Local::put(task);
+                            drop(prev);
+                            task = Local::take();
+                        }
+                        None => {}
+                    }
                 }
-            };
-
-            // It is assumed that all reasonable rust code will have a local
-            // task at all times. This means that this `try_unsafe_borrow` will
-            // succeed almost all of the time. There are border cases, however,
-            // when the runtime has *almost* set up the local task, but hasn't
-            // quite gotten there yet. In order to get some better diagnostics,
-            // we print on failure and immediately abort the whole process if
-            // there is no local task available.
-            match Local::try_unsafe_borrow() {
-                Some(t) => {
-                    task = t;
-                    let n = (*task).name.as_ref()
-                                   .map(|n| n.as_slice()).unwrap_or("<unnamed>");
-
+                None => {
                     rterrln!("task '{}' failed at '{}', {}:{}", n, msg_s,
                              file, line);
                 }
-                None => {
-                    rterrln!("failed at '{}', {}:{}", msg_s, file, line);
-                    intrinsics::abort();
-                }
-            }
-
-            if (*task).unwinder.unwinding {
-                // If a task fails while it's already unwinding then we
-                // have limited options. Currently our preference is to
-                // just abort. In the future we may consider resuming
-                // unwinding or otherwise exiting the task cleanly.
-                rterrln!("task failed during unwinding (double-failure - total drag!)")
-                rterrln!("rust must abort now. so sorry.");
-                intrinsics::abort();
             }
         }
+        task.name = name;
+
+        if task.unwinder.unwinding {
+            // If a task fails while it's already unwinding then we
+            // have limited options. Currently our preference is to
+            // just abort. In the future we may consider resuming
+            // unwinding or otherwise exiting the task cleanly.
+            rterrln!("task failed during unwinding (double-failure - total drag!)")
+            rterrln!("rust must abort now. so sorry.");
+            unsafe { intrinsics::abort() }
+        }
+    }
 
+    // The unwinder won't actually use the task at all, so we put the task back
+    // into TLS right before we invoke the unwinder, but this means we need an
+    // unsafe reference back to the unwinder once it's in TLS.
+    Local::put(task);
+    unsafe {
+        let task: *mut Task = Local::unsafe_borrow();
         (*task).unwinder.begin_unwind(msg);
     }
 }