Fix concurrent writing

Actually, it might be that all that effort was for nought: I've just
realized we still have sync issues since out write calls are not for
isolated chunks of stuff but often very many for one `.print` call.
Thus, we need to lock the target in `.print`.

We could probably do that off-thread by putting the item into a channel,
but that'd require the item to implement `Send`. I'm not sure I want to
do that (robustness principle and all that). I might still evaluate this
in a spike branch just because I don't have enough experience to judge
the effects of this.

This commit surely adds a bit of tech debt, as we `Output` is now
basically a `Vec<Arc<Mutex<Arc<InternalFormatter>>>`. Sorry.

Author: killercup

src/error.rs

@@ -17,6 +17,9 @@ pub enum Error {
     /// Error in formatting worker
     #[fail(display = "{}", _0)]
     WorkerError(String),
+    /// Error syncing output
+    #[fail(display = "Error syncing output")]
+    SyncError,
 }
 
 impl From<io::Error> for Error {

src/human.rs

@@ -1,14 +1,14 @@
 //! Human output
 
-use std::sync::Arc;
+use std::sync::{Arc, Mutex};
 use termcolor::{ColorChoice, ColorSpec, StandardStream, WriteColor};
 use {Error, Target};
 
 /// Construct a new human output target that writes to stdout
 pub fn stdout() -> Result<Target, Error> {
-    Ok(Target::Human(Formatter::init_with(|| {
-        Ok(StandardStream::stdout(ColorChoice::Auto))
-    })?))
+    Ok(Target::Human(Arc::new(Mutex::new(Formatter::init_with(
+        || Ok(StandardStream::stdout(ColorChoice::Auto)),
+    )?))))
 }
 
 pub use self::test_helper::{test, test_with_color};
@@ -216,6 +216,7 @@ macro_rules! render_for_humans {
 
 mod test_helper {
     use super::Formatter;
+    use std::sync::{Arc, Mutex};
     use termcolor::Buffer;
     use {test_buffer::TestBuffer, Target};
 
@@ -268,7 +269,7 @@ mod test_helper {
         }
 
         pub fn target(&self) -> Target {
-            Target::Human(self.formatter())
+            Target::Human(Arc::new(Mutex::new(self.formatter())))
         }
 
         pub fn to_string(&self) -> String {

src/json.rs

@@ -4,32 +4,35 @@ use serde::Serialize;
 use serde_json::to_vec as write_json;
 use std::io::Write;
 use std::path::Path;
-use std::sync::Arc;
+use std::sync::{Arc, Mutex};
 use {Error, Target};
 
 /// Create a new JSON output that writes to a file
 pub fn file<T: AsRef<Path>>(name: T) -> Result<Target, Error> {
     let path = name.as_ref().to_path_buf();
-    Ok(Target::Json(Formatter::init_with(move || {
-        use std::fs::{File, OpenOptions};
-        use std::io::BufWriter;
-
-        let target = if path.exists() {
-            let mut f = OpenOptions::new().write(true).append(true).open(&path)?;
-            f.write_all(b"\n")?;
-
-            f
-        } else {
-            File::create(&path)?
-        };
+    Ok(Target::Json(Arc::new(Mutex::new(Formatter::init_with(
+        move || {
+            use std::fs::{File, OpenOptions};
+            use std::io::BufWriter;
+
+            let target = if path.exists() {
+                let mut f = OpenOptions::new().write(true).append(true).open(&path)?;
+                f.write_all(b"\n")?;
+
+                f
+            } else {
+                File::create(&path)?
+            };
 
-        Ok(BufWriter::new(target))
-    })?))
+            Ok(BufWriter::new(target))
+        },
+    )?))))
 }
 
 pub use self::test_helper::test;
 
 /// JSON formatter
+#[derive(Clone)]
 pub struct Formatter {
     inner: Arc<InternalFormatter>,
 }
@@ -45,9 +48,7 @@ impl Formatter {
 
     /// Write a serializable item to the JSON formatter
     pub fn write<T: Serialize>(&self, item: &T) -> Result<(), Error> {
-        let mut bytes = write_json(item)?;
-        bytes.push(b'\n');
-        self.send(Message::Write(bytes));
+        self.send(Message::Write(write_json(item)?));
         Ok(())
     }
 
@@ -206,6 +207,7 @@ macro_rules! render_json {
 
 mod test_helper {
     use super::Formatter;
+    use std::sync::{Arc, Mutex};
     use termcolor::Buffer;
     use test_buffer::TestBuffer;
     use Target;
@@ -262,7 +264,7 @@ mod test_helper {
         }
 
         pub fn target(&self) -> Target {
-            Target::Json(self.formatter())
+            Target::Json(Arc::new(Mutex::new(self.formatter())))
         }
 
         pub fn to_string(&self) -> String {

src/lib.rs

@@ -38,7 +38,7 @@ pub fn new() -> Output {
 }
 
 /// Structure holding your output targets
-#[derive(Default)]
+#[derive(Default, Clone)]
 pub struct Output {
     targets: Vec<Target>,
 }
@@ -57,16 +57,19 @@ fn assert_output_is_sync_and_send() {
     assert_both::<Output>();
 }
 
+use std::sync::{Arc, Mutex};
+
 /// Known targets to write to
+#[derive(Clone)]
 pub enum Target {
     /// Human readable output
     ///
     /// Will mostly be (unstructured) text, optionally with formatting.
-    Human(human::Formatter),
+    Human(Arc<Mutex<human::Formatter>>),
     /// JSON output
     ///
     /// Machines like this.
-    Json(json::Formatter),
+    Json(Arc<Mutex<json::Formatter>>),
 }
 
 mod error;
@@ -78,11 +81,13 @@ impl Output {
         for target in &mut self.targets {
             match target {
                 Target::Human(fmt) => {
-                    item.render_for_humans(fmt)?;
+                    let mut fmt = fmt.lock().map_err(|_| Error::SyncError)?;
+                    item.render_for_humans(&mut *fmt)?;
                     fmt.write("\n")?;
                 }
                 Target::Json(fmt) => {
-                    item.render_json(fmt)?;
+                    let mut fmt = fmt.lock().map_err(|_| Error::SyncError)?;
+                    item.render_json(&mut *fmt)?;
                     fmt.write_separator()?;
                 }
             }
@@ -96,9 +101,11 @@ impl Output {
         for target in &self.targets {
             match target {
                 Target::Human(fmt) => {
+                    let mut fmt = fmt.lock().map_err(|_| Error::SyncError)?;
                     fmt.flush()?;
                 }
                 Target::Json(fmt) => {
+                    let mut fmt = fmt.lock().map_err(|_| Error::SyncError)?;
                     fmt.flush()?;
                 }
             }
@@ -176,6 +183,33 @@ impl<'a> Render for &'a str {
     }
 }
 
+/// Render a string
+///
+/// # Examples
+///
+/// ```rust
+/// # extern crate output;
+/// # use output::human;
+/// # fn main() -> Result<(), output::Error> {
+/// # let test_target = human::test();
+/// let mut out = output::new().add_target(test_target.target());
+/// out.print(String::from("Hello, World!"))?;
+/// # out.flush()?;
+/// # assert_eq!(test_target.to_string(), "Hello, World!\n");
+/// # Ok(()) }
+/// ```
+impl<'a> Render for String {
+    fn render_for_humans(&self, fmt: &mut human::Formatter) -> Result<(), Error> {
+        fmt.write(self.as_bytes())?;
+        Ok(())
+    }
+
+    fn render_json(&self, fmt: &mut json::Formatter) -> Result<(), Error> {
+        fmt.write(&self)?;
+        Ok(())
+    }
+}
+
 pub mod components;
 pub mod human;
 pub mod json;