Skip to content

Commit 9fd3eff

Browse files
committed
resolve print race which prevents proper error message from being displayed sometimes
1 parent 43052c5 commit 9fd3eff

File tree

3 files changed

+30
-38
lines changed

3 files changed

+30
-38
lines changed

src/main.rs

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -10,16 +10,13 @@
1010
clippy::style,
1111
clippy::suspicious
1212
)]
13-
#![allow(
14-
clippy::cast_precision_loss,
15-
clippy::struct_excessive_bools,
16-
)]
13+
#![allow(clippy::cast_precision_loss, clippy::struct_excessive_bools)]
1714

1815
use clap::CommandFactory;
1916
use context::{layout, Context};
2017
use progress::{Indicator, IndicatorHandle, Message};
2118
use render::{Engine, Flat, FlatInverted, Inverted, Regular};
22-
use std::{error::Error, io::stdout, process::ExitCode, sync::Arc};
19+
use std::{error::Error, io::stdout, process::ExitCode};
2320
use tree::Tree;
2421

2522
/// Operations to wrangle ANSI escaped strings.
@@ -78,7 +75,7 @@ fn run() -> Result<(), Box<dyn Error>> {
7875
let indicator = Indicator::maybe_init(&ctx);
7976

8077
let (tree, ctx) = {
81-
match Tree::try_init(ctx, indicator.clone()) {
78+
match Tree::try_init(ctx, indicator.as_ref()) {
8279
Ok(res) => res,
8380
Err(err) => {
8481
IndicatorHandle::terminate(indicator);
@@ -104,12 +101,11 @@ fn run() -> Result<(), Box<dyn Error>> {
104101
if let Some(mut progress) = indicator {
105102
progress.mailbox().send(Message::RenderReady)?;
106103

107-
if let Some(hand) = Arc::get_mut(&mut progress) {
108-
hand.join_handle
109-
.take()
110-
.map(|h| h.join().unwrap())
111-
.transpose()?;
112-
}
104+
progress
105+
.join_handle
106+
.take()
107+
.map(|h| h.join().unwrap())
108+
.transpose()?;
113109
}
114110

115111
#[cfg(debug_assertions)]

src/progress.rs

Lines changed: 18 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,7 @@ use crossterm::{
66
};
77
use std::{
88
io::{self, Write},
9-
sync::{
10-
mpsc::{self, SendError, SyncSender},
11-
Arc,
12-
},
9+
sync::mpsc::{self, SendError, SyncSender},
1310
thread::{self, JoinHandle},
1411
};
1512

@@ -101,22 +98,25 @@ impl IndicatorHandle {
10198
}
10299

103100
/// Analogous to [`Self::try_terminate`] but panics if failure.
104-
pub fn terminate(this: Option<Arc<Self>>) {
101+
pub fn terminate(this: Option<Self>) {
105102
Self::try_terminate(this).expect("Failed to properly terminate the progress indicator");
106103
}
107104

108105
/// Attempts to terminate the [`Indicator`] with cleanup.
109-
pub fn try_terminate(this: Option<Arc<Self>>) -> Result<(), Error> {
106+
pub fn try_terminate(this: Option<Self>) -> Result<(), Error> {
110107
if let Some(mut handle) = this {
111-
eprintln!("{}", Arc::strong_count(&handle));
112-
handle.mailbox().send(Message::Finish)?;
113-
114-
if let Some(hand) = Arc::get_mut(&mut handle) {
115-
hand.join_handle
116-
.take()
117-
.map(|h| h.join().unwrap())
118-
.transpose()?;
119-
}
108+
// This is allowed to fail silently. If user administers interrupt then the `Indicator`
109+
// will be dropped along with the receiving end of the `mailbox`.
110+
//
111+
// If user does not administer interrupt but file-system traversal fails for whatever
112+
// reason then this will proceed as normal.
113+
let _ = handle.mailbox().send(Message::Finish);
114+
115+
handle
116+
.join_handle
117+
.take()
118+
.map(|h| h.join().unwrap())
119+
.transpose()?;
120120
}
121121

122122
Ok(())
@@ -128,15 +128,14 @@ impl<'a> Indicator<'a> {
128128
/// a progress indicator is enabled via [`Context`]. Upon initialization an interrupt handler is
129129
/// also registered. Sources of panic can come from [`IndicatorHandle::terminate`] or
130130
/// [`ctrlc::set_handler`].
131-
pub fn maybe_init(ctx: &Context) -> Option<Arc<IndicatorHandle>> {
131+
pub fn maybe_init(ctx: &Context) -> Option<IndicatorHandle> {
132132
(ctx.stdout_is_tty && !ctx.no_progress)
133133
.then(Indicator::measure)
134134
.map(|indicator| {
135-
let indicator = Arc::new(indicator);
136-
let arc = indicator.clone();
135+
let mailbox = indicator.mailbox();
137136

138137
let int_handler = move || {
139-
IndicatorHandle::terminate(Some(arc.clone()));
138+
let _ = mailbox.try_send(Message::Finish);
140139
tty::restore_tty();
141140
};
142141

src/tree/mod.rs

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,7 @@ use std::{
1616
fs,
1717
path::PathBuf,
1818
result::Result as StdResult,
19-
sync::{
20-
mpsc::{self, Sender},
21-
Arc,
22-
},
19+
sync::mpsc::{self, Sender},
2320
thread,
2421
};
2522
use visitor::{BranchVisitorBuilder, TraversalState};
@@ -54,7 +51,7 @@ impl Tree {
5451
/// various properties necessary to render output.
5552
pub fn try_init(
5653
mut ctx: Context,
57-
indicator: Option<Arc<IndicatorHandle>>,
54+
indicator: Option<&IndicatorHandle>,
5855
) -> Result<(Self, Context)> {
5956
let mut column_properties = column::Properties::from(&ctx);
6057

@@ -102,12 +99,12 @@ impl Tree {
10299
fn traverse(
103100
ctx: &Context,
104101
column_properties: &mut column::Properties,
105-
indicator: Option<Arc<IndicatorHandle>>,
102+
indicator: Option<&IndicatorHandle>,
106103
) -> Result<(Arena<Node>, NodeId)> {
107104
let walker = WalkParallel::try_from(ctx)?;
108105
let (tx, rx) = mpsc::channel();
109106

110-
let progress_indicator_mailbox = indicator.map(|arc| arc.mailbox());
107+
let progress_indicator_mailbox = indicator.map(IndicatorHandle::mailbox);
111108

112109
thread::scope(|s| {
113110
let res = s.spawn(move || {

0 commit comments

Comments
 (0)