Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Stacked PR] Remove warn_later from output #760

Merged
merged 2 commits into from
Jan 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
213 changes: 213 additions & 0 deletions libherokubuildpack/src/output/background.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,213 @@
use std::fmt::Debug;
use std::io::Write;
use std::sync::mpsc::channel;
use std::time::Duration;

/// This module is responsible for the logic involved in the printing to output while
/// other work is being performed. Such as printing dots while a download is being performed

/// Print dots to the given buffer at the given interval
///
/// Returns a struct that allows for manually stopping the timer or will automatically stop
/// the timer if the guard is dropped. This functionality allows for errors that trigger
/// an exit of the function to not accidentally have a timer printing in the background
/// forever.
#[must_use]
pub(crate) fn print_interval<W>(
mut buffer: W,
interval: Duration,
start: String,
tick: String,
end: String,
) -> state::PrintGuard<W>
where
W: Write + Send + Debug + 'static,
{
let (sender, receiver) = channel::<()>();

let join_handle = std::thread::spawn(move || {
write!(buffer, "{start}").expect("Internal error");
buffer.flush().expect("Internal error");

loop {
write!(buffer, "{tick}").expect("Internal error");
buffer.flush().expect("Internal error");

if receiver.recv_timeout(interval).is_ok() {
break;
}
}

write!(buffer, "{end}").expect("Internal error");
buffer.flush().expect("Internal error");

buffer
});

state::PrintGuard::new(join_handle, sender)
}

pub(crate) mod state {
use std::fmt::Debug;
use std::io::Write;
use std::sync::mpsc::Sender;
use std::thread::JoinHandle;

/// Holds the reference to the background printer
///
/// Ensures that the dot printer is stopped in the event of an error. By signaling
/// it and joining when this struct is dropped.
///
/// Gives access to the original io/buffer struct passed to the background writer
/// when the thread is manually stopped.
///
/// # Panics
///
/// Updates to this code need to take care to not introduce a panic. See
/// documentation in `PrintGuard::stop` below for more details.
#[derive(Debug)]
pub(crate) struct PrintGuard<W>
where
W: Write + Debug,
{
/// Holds the handle to the thread printing dots
///
/// Structs that implement `Drop` must ensure a valid internal state at
/// all times due to E0509. The handle is wrapped in an option to allow the
/// inner value to be removed while preserving internal state.
join_handle: Option<JoinHandle<W>>,

/// Holds the signaling method to tell the background printer
/// to stop emitting.
stop_signal: Sender<()>,
}

impl<W> Drop for PrintGuard<W>
where
W: Write + Debug,
{
fn drop(&mut self) {
// A note on correctness. It might seem that it's enough to signal the thread to
// stop, that we don't also have to join and wait for it to finish, but that's not
// the case. The printer can still emit a value after it's told to stop.
//
// When that happens the output can appear in the middle of another output, such
// as an error message if a global writer is being used such as stdout.
// As a result we have to signal AND ensure the thread is stoped before
// continuing.
if let Some(join_handle) = self.join_handle.take() {
let _ = self.stop_signal.send(());
let _ = join_handle.join();
}
}
}

impl<W> PrintGuard<W>
where
W: Write + Debug,
{
/// Preserve internal state by ensuring the `Option` is always populated
pub(crate) fn new(join_handle: JoinHandle<W>, sender: Sender<()>) -> Self {
let guard = PrintGuard {
join_handle: Some(join_handle),
stop_signal: sender,
};
debug_assert!(guard.join_handle.is_some());

guard
}

/// The only thing a consumer can do is stop the dots and receive
/// the original buffer.
///
/// # Panics
///
/// This code can panic if it encounters an unexpedcted internal state.
/// If that happens it means there is an internal bug in this logic.
/// To avoid a panic, developers modifying this file must obey the following
/// rules:
///
/// - Always consume the struct when accessing the Option value outside of Drop
/// - Never construct this struct with a `None` option value.
///
/// This `Option` wrapping is needed to support a implementing Drop to
/// ensure the printing is stopped. When a struct implements drop it cannot
/// remove it's internal state due to E0509:
///
/// <https://github.com/rust-lang/rust/blob/27d8a577138c0d319a572cd1a464c2b755e577de/compiler/rustc_error_codes/src/error_codes/E0509.md>
///
/// The workaround is to never allow invalid internal state by replacing the
/// inner value with a `None` when removing it. We don't want to expose this
/// implementation detail to the user, so instead we accept the panic, ensure
/// the code is exercised under test, and exhaustively document why this panic
/// exists and how developers working with this code can maintain safety.
#[allow(clippy::panic_in_result_fn)]
pub(crate) fn stop(mut self) -> std::thread::Result<W> {
// Ignore if the channel is closed, likely means the thread died which
// we want in this case.
match self.join_handle.take() {
Some(join_handle) => {
let _ = self.stop_signal.send(());
join_handle.join()
}
None => panic!("Internal error: Dot print internal state should never be None"),
}
}
}
}

#[cfg(test)]
mod test {
use super::*;
use std::fs::{File, OpenOptions};
use tempfile::NamedTempFile;

#[test]
fn does_stop_does_not_panic() {
let mut buffer: Vec<u8> = vec![];
write!(buffer, "before").unwrap();

let dot = print_interval(
buffer,
Duration::from_millis(1),
String::from(" ."),
String::from("."),
String::from(". "),
);
let mut writer = dot.stop().unwrap();

write!(writer, "after").unwrap();
writer.flush().unwrap();

assert_eq!("before ... after", String::from_utf8_lossy(&writer));
}

#[test]
fn test_drop_stops_timer() {
let tempfile = NamedTempFile::new().unwrap();
let mut log = File::create(tempfile.path()).unwrap();
write!(log, "before").unwrap();

let dot = print_interval(
log,
Duration::from_millis(1),
String::from(" ."),
String::from("."),
String::from(". "),
);
drop(dot);

let mut log = OpenOptions::new()
.append(true)
.write(true)
.open(tempfile.path())
.unwrap();
write!(log, "after").unwrap();
log.flush().unwrap();

assert_eq!(
String::from("before ... after"),
std::fs::read_to_string(tempfile.path()).unwrap()
);
}
}
157 changes: 0 additions & 157 deletions libherokubuildpack/src/output/background_timer.rs

This file was deleted.

Loading