Skip to content

Fixed race to print #220

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

Merged
merged 2 commits into from
Jul 2, 2023
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
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,7 @@ $ rustup toolchain install nightly-2023-06-11
Thereafter:

```
$ cargo +nightly-2023-03-05 install erdtree
$ cargo +nightly-2023-06-11 install erdtree
```

### Homebrew-core
Expand Down
6 changes: 4 additions & 2 deletions src/context/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -257,8 +257,10 @@ impl Context {
/// Initializes [Context], optionally reading in the configuration file to override defaults.
/// Arguments provided will take precedence over config.
pub fn try_init() -> Result<Self, Error> {
let args = Self::compute_args()?;
Self::from_arg_matches(&args).map_err(Error::Config)
Self::compute_args().and_then(|args| {
color::no_color_env();
Self::from_arg_matches(&args).map_err(Error::Config)
})
}

/// Determines whether or not it's appropriate to display color in output based on
Expand Down
58 changes: 18 additions & 40 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,20 +10,13 @@
clippy::style,
clippy::suspicious
)]
#![allow(
clippy::cast_possible_truncation,
clippy::cast_precision_loss,
clippy::cast_sign_loss,
clippy::let_underscore_untyped,
clippy::struct_excessive_bools,
clippy::too_many_arguments
)]
#![allow(clippy::cast_precision_loss, clippy::struct_excessive_bools)]

use clap::CommandFactory;
use context::{layout, Context};
use progress::Message;
use progress::{Indicator, IndicatorHandle, Message};
use render::{Engine, Flat, FlatInverted, Inverted, Regular};
use std::{error::Error, io::stdout, process::ExitCode, sync::Arc};
use std::{error::Error, io::stdout, process::ExitCode};
use tree::Tree;

/// Operations to wrangle ANSI escaped strings.
Expand All @@ -44,10 +37,7 @@ mod icons;
/// Concerned with displaying a progress indicator when stdout is a tty.
mod progress;

/// Concerned with taking an initialized [`Tree`] and its [`Node`]s and rendering the output.
///
/// [`Tree`]: tree::Tree
/// [`Node`]: tree::node::Node
/// Concerned with taking an initialized [`tree::Tree`] and its [`tree::node::Node`]s and rendering the output.
mod render;

/// Global used throughout the program to paint the output.
Expand Down Expand Up @@ -80,29 +70,18 @@ fn run() -> Result<(), Box<dyn Error>> {
return Ok(());
}

context::color::no_color_env();

styles::init(ctx.no_color());

let indicator = (ctx.stdout_is_tty && !ctx.no_progress)
.then(progress::Indicator::measure)
.map(Arc::new);
let indicator = Indicator::maybe_init(&ctx);

if indicator.is_some() {
let indicator = indicator.clone();

ctrlc::set_handler(move || {
let _ = progress::IndicatorHandle::terminate(indicator.clone());
tty::restore_tty();
})?;
}

let (tree, ctx) = match Tree::try_init(ctx, indicator.clone()) {
Ok(res) => res,
Err(err) => {
let _ = progress::IndicatorHandle::terminate(indicator);
return Err(Box::new(err));
},
let (tree, ctx) = {
match Tree::try_init(ctx, indicator.as_ref()) {
Ok(res) => res,
Err(err) => {
IndicatorHandle::terminate(indicator);
return Err(Box::new(err));
},
}
};

macro_rules! compute_output {
Expand All @@ -122,12 +101,11 @@ fn run() -> Result<(), Box<dyn Error>> {
if let Some(mut progress) = indicator {
progress.mailbox().send(Message::RenderReady)?;

if let Some(hand) = Arc::get_mut(&mut progress) {
hand.join_handle
.take()
.map(|h| h.join().unwrap())
.transpose()?;
}
progress
.join_handle
.take()
.map(|h| h.join().unwrap())
.transpose()?;
}

#[cfg(debug_assertions)]
Expand Down
56 changes: 42 additions & 14 deletions src/progress.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,12 @@
use crate::{context::Context, tty};
use crossterm::{
cursor,
terminal::{self, ClearType},
ExecutableCommand,
};
use std::{
io::{self, Write},
sync::{
mpsc::{self, SendError, SyncSender},
Arc,
},
sync::mpsc::{self, SendError, SyncSender},
thread::{self, JoinHandle},
};

Expand Down Expand Up @@ -99,24 +97,54 @@ impl IndicatorHandle {
self.mailbox.clone()
}

/// Send a message through to the `priority_mailbox` tear down the [`Indicator`].
pub fn terminate(this: Option<Arc<Self>>) -> Result<(), Error> {
if let Some(mut handle) = this {
handle.mailbox().send(Message::Finish)?;
/// Analogous to [`Self::try_terminate`] but panics if failure.
pub fn terminate(this: Option<Self>) {
Self::try_terminate(this).expect("Failed to properly terminate the progress indicator");
}

if let Some(hand) = Arc::get_mut(&mut handle) {
hand.join_handle
.take()
.map(|h| h.join().unwrap())
.transpose()?;
}
/// Attempts to terminate the [`Indicator`] with cleanup.
pub fn try_terminate(this: Option<Self>) -> Result<(), Error> {
if let Some(mut handle) = this {
// This is allowed to fail silently. If user administers interrupt then the `Indicator`
// will be dropped along with the receiving end of the `mailbox`.
//
// If user does not administer interrupt but file-system traversal fails for whatever
// reason then this will proceed as normal.
let _ = handle.mailbox().send(Message::Finish);

handle
.join_handle
.take()
.map(|h| h.join().unwrap())
.transpose()?;
}

Ok(())
}
}

impl<'a> Indicator<'a> {
/// Initializes an [`Indicator`] returning an atomic reference counter of an [`IndicatorHandle`] if
/// a progress indicator is enabled via [`Context`]. Upon initialization an interrupt handler is
/// also registered. Sources of panic can come from [`IndicatorHandle::terminate`] or
/// [`ctrlc::set_handler`].
pub fn maybe_init(ctx: &Context) -> Option<IndicatorHandle> {
(ctx.stdout_is_tty && !ctx.no_progress)
.then(Indicator::measure)
.map(|indicator| {
let mailbox = indicator.mailbox();

let int_handler = move || {
let _ = mailbox.try_send(Message::Finish);
tty::restore_tty();
};

ctrlc::set_handler(int_handler).expect("Failed to set interrupt handler");

indicator
})
}

/// Initializes a worker thread that owns [`Indicator`] that awaits on [`Message`]s to traverse
/// through its internal states. An [`IndicatorHandle`] is returned as a mechanism to allow the
/// outside world to send messages to the worker thread and ultimately to the [`Indicator`].
Expand Down
16 changes: 5 additions & 11 deletions src/tree/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,7 @@ use std::{
fs,
path::PathBuf,
result::Result as StdResult,
sync::{
mpsc::{self, Sender},
Arc,
},
sync::mpsc::{self, Sender},
thread,
};
use visitor::{BranchVisitorBuilder, TraversalState};
Expand All @@ -30,10 +27,7 @@ pub mod count;
/// Errors related to traversal, [Tree] construction, and the like.
pub mod error;

/// Contains components of the [`Tree`] data structure that derive from [`DirEntry`].
///
/// [`Tree`]: Tree
/// [`DirEntry`]: ignore::DirEntry
/// Contains components of the [`Tree`] data structure that derive from [`ignore::DirEntry`].
pub mod node;

/// Custom visitor that operates on each thread during filesystem traversal.
Expand All @@ -57,7 +51,7 @@ impl Tree {
/// various properties necessary to render output.
pub fn try_init(
mut ctx: Context,
indicator: Option<Arc<IndicatorHandle>>,
indicator: Option<&IndicatorHandle>,
) -> Result<(Self, Context)> {
let mut column_properties = column::Properties::from(&ctx);

Expand Down Expand Up @@ -105,12 +99,12 @@ impl Tree {
fn traverse(
ctx: &Context,
column_properties: &mut column::Properties,
indicator: Option<Arc<IndicatorHandle>>,
indicator: Option<&IndicatorHandle>,
) -> Result<(Arena<Node>, NodeId)> {
let walker = WalkParallel::try_from(ctx)?;
let (tx, rx) = mpsc::channel();

let progress_indicator_mailbox = indicator.map(|arc| arc.mailbox());
let progress_indicator_mailbox = indicator.map(IndicatorHandle::mailbox);

thread::scope(|s| {
let res = s.spawn(move || {
Expand Down