Don't read stdout into memory when not needed

.gitignore

@@ -1,2 +1,2 @@
 /target/
-Cargo.lock
+/Cargo.lock

Cargo.toml

@@ -10,6 +10,9 @@ homepage = "https://github.com/soenkehahn/cradle"
 keywords = ["child", "child-process", "command", "process", "shell"]
 categories = ["filesystem", "os"]
 
+[workspace]
+members = [".", "memory-test"]
+
 [dependencies]
 rustversion = "1.0.4"
 

justfile

@@ -1,10 +1,10 @@
 ci: test build doc clippy fmt context-integration-tests run-examples forbidden-words render-readme-check
 
 build:
-  cargo build --all-targets --all-features
+  cargo build --all-targets --all-features --workspace
 
 test +pattern="":
-  cargo test --all {{ pattern }}
+  cargo test {{ pattern }}
 
 test-lib-fast +pattern="":
   cargo test --lib {{ pattern }}
@@ -13,10 +13,10 @@ context-integration-tests:
   cargo run --features "test_executables" --bin context_integration_tests
 
 doc +args="":
-  cargo doc --all {{args}}
+  cargo doc --workspace {{args}}
 
 clippy:
-  cargo clippy --all-targets --all-features
+  cargo clippy --all-targets --all-features --workspace
 
 fmt:
   cargo fmt --all -- --check

memory-test/Cargo.toml

@@ -0,0 +1,9 @@
+[package]
+name = "memory-test"
+version = "0.0.0"
+edition = "2018"
+
+[dependencies]
+anyhow = "1.0.42"
+cradle = { path = ".." }
+rustversion = "1.0.5"

memory-test/src/bin/cradle_user.rs

@@ -0,0 +1,8 @@
+use cradle::prelude::*;
+
+fn main() {
+    let mut args = std::env::args();
+    let bytes: usize = args.nth(1).unwrap().parse().unwrap();
+    eprintln!("consuming {} kB", bytes / 2_usize.pow(10));
+    cmd_unit!("./target/release/produce_bytes", bytes.to_string());
+}

memory-test/src/bin/produce_bytes.rs

@@ -0,0 +1,21 @@
+use anyhow::Result;
+use std::io::{stdout, Write};
+
+fn main() -> Result<()> {
+    let mut args = std::env::args();
+    let mut bytes: usize = args.nth(1).unwrap().parse()?;
+    eprintln!("producing {} kB", bytes / 2_usize.pow(10));
+    let buffer = &[b'x'; 1024];
+    let mut stdout = stdout();
+    while bytes > 0 {
+        if bytes >= buffer.len() {
+            stdout.write_all(buffer)?;
+            bytes -= buffer.len();
+        } else {
+            stdout.write_all(&[b'x'])?;
+            bytes -= 1;
+        }
+    }
+    stdout.flush()?;
+    Ok(())
+}

memory-test/src/bin/run_test.rs

@@ -0,0 +1,56 @@
+use anyhow::Result;
+use cradle::prelude::*;
+use std::process::{Command, Stdio};
+
+fn from_mb(mega_bytes: usize) -> usize {
+    mega_bytes * 2_usize.pow(20)
+}
+
+fn main() -> Result<()> {
+    Split("cargo build --release").run_unit();
+    let bytes = from_mb(64);
+    let memory_consumption = measure_memory_consumption(bytes)?;
+    let allowed_memory_consumption = from_mb(16);
+    assert!(
+        memory_consumption < allowed_memory_consumption,
+        "Maximum resident set size: {}, allowed upper limit: {}",
+        memory_consumption,
+        allowed_memory_consumption
+    );
+    Ok(())
+}
+
+fn measure_memory_consumption(bytes: usize) -> Result<usize> {
+    let output = Command::new("/usr/bin/time")
+        .arg("-v")
+        .arg("./target/release/cradle_user")
+        .arg(bytes.to_string())
+        .stdout(Stdio::null())
+        .output()?;
+    let stderr = String::from_utf8(output.stderr)?;
+    eprintln!("{}", stderr);
+    if !output.status.success() {
+        panic!("running 'cradle_user' failed");
+    }
+    let memory_size_prefix = "Maximum resident set size (kbytes): ";
+    let kilo_bytes: usize = strip_prefix(
+        stderr
+            .lines()
+            .map(|line| line.trim())
+            .find(|line| line.starts_with(memory_size_prefix))
+            .unwrap(),
+        memory_size_prefix,
+    )
+    .parse()?;
+    let bytes = kilo_bytes * 1024;
+    Ok(bytes)
+}
+
+#[rustversion::attr(since(1.48), allow(clippy::manual_strip))]
+fn strip_prefix<'a>(string: &'a str, prefix: &'a str) -> &'a str {
+    if string.starts_with(prefix) {
+        &string[prefix.len()..]
+    } else {
+        panic!("{} doesn't start with {}", string, prefix);
+    }
+}

src/collected_output.rs

@@ -7,7 +7,7 @@ use std::{
 
 pub(crate) struct Waiter {
     stdin: JoinHandle<io::Result<()>>,
-    stdout: JoinHandle<io::Result<Vec<u8>>>,
+    stdout: JoinHandle<io::Result<Option<Vec<u8>>>>,
     stderr: JoinHandle<io::Result<Vec<u8>>>,
 }
 
@@ -30,8 +30,8 @@ impl Waiter {
         });
         let mut context_clone = context.clone();
         let relay_stdout = config.relay_stdout;
-        let stdout_join_handle = thread::spawn(move || -> io::Result<Vec<u8>> {
-            let mut collected_stdout = Vec::new();
+        let stdout_join_handle = thread::spawn(move || -> io::Result<Option<Vec<u8>>> {
+            let mut collected_stdout = if relay_stdout { None } else { Some(Vec::new()) };
             let buffer = &mut [0; 256];
             loop {
                 let length = child_stdout.read(buffer)?;
@@ -41,7 +41,9 @@ impl Waiter {
                 if relay_stdout {
                     context_clone.stdout.write_all(&buffer[..length])?;
                 }
-                collected_stdout.extend(&buffer[..length]);
+                if let Some(collected_stdout) = &mut collected_stdout {
+                    collected_stdout.extend(&buffer[..length]);
+                }
             }
             Ok(collected_stdout)
         });
@@ -87,6 +89,6 @@ impl Waiter {
 }
 
 pub(crate) struct CollectedOutput {
-    pub(crate) stdout: Vec<u8>,
+    pub(crate) stdout: Option<Vec<u8>>,
     pub(crate) stderr: Vec<u8>,
 }

src/config.rs

@@ -4,6 +4,7 @@ use std::{ffi::OsString, path::PathBuf, sync::Arc};
 
 #[doc(hidden)]
 #[rustversion::attr(since(1.48), allow(clippy::rc_buffer))]
+#[derive(Debug, Clone)]
 pub struct Config {
     pub(crate) arguments: Vec<OsString>,
     pub(crate) log_command: bool,

src/error.rs

@@ -26,6 +26,10 @@ pub enum Error {
         full_command: String,
         source: Arc<FromUtf8Error>,
     },
+    CradleBug {
+        full_command: String,
+        config: Config,
+    },
 }
 
 impl Error {
@@ -35,6 +39,13 @@ impl Error {
             source: Arc::new(error),
         }
     }
+
+    pub(crate) fn cradle_bug(config: &Config) -> Error {
+        Error::CradleBug {
+            full_command: config.full_command(),
+            config: config.clone(),
+        }
+    }
 }
 
 #[doc(hidden)]
@@ -48,15 +59,16 @@ pub fn panic_on_error<T>(result: Result<T, Error>) -> T {
 
 impl Display for Error {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        use Error::*;
         match self {
-            Error::NoArgumentsGiven => write!(f, "no arguments given"),
-            Error::FileNotFoundWhenExecuting { executable, .. } => write!(
+            NoArgumentsGiven => write!(f, "no arguments given"),
+            FileNotFoundWhenExecuting { executable, .. } => write!(
                 f,
                 "File not found error when executing '{}'",
                 executable.to_string_lossy()
             ),
-            Error::CommandIoError { message, .. } => write!(f, "{}", message),
-            Error::NonZeroExitCode {
+            CommandIoError { message, .. } => write!(f, "{}", message),
+            NonZeroExitCode {
                 full_command,
                 exit_status,
             } => {
@@ -70,24 +82,35 @@ impl Display for Error {
                     write!(f, "{}:\n  exited with {}", full_command, exit_status)
                 }
             }
-            Error::InvalidUtf8ToStdout { full_command, .. } => {
+            InvalidUtf8ToStdout { full_command, .. } => {
                 write!(f, "{}:\n  invalid utf-8 written to stdout", full_command)
             }
-            Error::InvalidUtf8ToStderr { full_command, .. } => {
+            InvalidUtf8ToStderr { full_command, .. } => {
                 write!(f, "{}:\n  invalid utf-8 written to stderr", full_command)
             }
+            CradleBug { .. } => {
+                let snippets = vec![
+                  "Congratulations, you've found a bug in cradle! :/",
+                  "Please, consider reporting a bug on https://github.com/soenkehahn/cradle/issues,",
+                  "including the following information:",
+                ];
+                writeln!(f, "{}\n{:#?}", snippets.join(" "), self)
+            }
         }
     }
 }
 
 impl std::error::Error for Error {
     fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
+        use Error::*;
         match self {
-            Error::FileNotFoundWhenExecuting { source, .. }
-            | Error::CommandIoError { source, .. } => Some(&**source),
-            Error::InvalidUtf8ToStdout { source, .. }
-            | Error::InvalidUtf8ToStderr { source, .. } => Some(&**source),
-            Error::NoArgumentsGiven | Error::NonZeroExitCode { .. } => None,
+            FileNotFoundWhenExecuting { source, .. } | CommandIoError { source, .. } => {
+                Some(&**source)
+            }
+            InvalidUtf8ToStdout { source, .. } | InvalidUtf8ToStderr { source, .. } => {
+                Some(&**source)
+            }
+            NoArgumentsGiven | NonZeroExitCode { .. } | CradleBug { .. } => None,
         }
     }
 }

src/lib.rs

@@ -320,7 +320,7 @@ where
 #[doc(hidden)]
 #[derive(Clone)]
 pub struct RunResult {
-    stdout: Vec<u8>,
+    stdout: Option<Vec<u8>>,
     stderr: Vec<u8>,
     exit_status: ExitStatus,
 }

src/output.rs

@@ -171,7 +171,8 @@ impl Output for StdoutUntrimmed {
     #[doc(hidden)]
     fn from_run_result(config: &Config, result: Result<RunResult, Error>) -> Result<Self, Error> {
         let result = result?;
-        Ok(StdoutUntrimmed(String::from_utf8(result.stdout).map_err(
+        let stdout = result.stdout.ok_or_else(|| Error::cradle_bug(config))?;
+        Ok(StdoutUntrimmed(String::from_utf8(stdout).map_err(
             |source| Error::InvalidUtf8ToStdout {
                 full_command: config.full_command(),
                 source: Arc::new(source),

tests/integration.rs

@@ -313,3 +313,10 @@ mod run_interface {
         );
     }
 }
+
+#[test]
+fn memory_test() {
+    use cradle::prelude::*;
+    cmd_unit!(%"cargo build -p memory-test --release");
+    cmd_unit!(%"cargo run -p memory-test --bin run_test");
+}