Repository: sharkdp/fd (language: rust)

Issue:
--max-results=1 does not actually quit after the first result
**Describe the bug you encountered:**

I am running `fd` on a slow FUSE filesystem ([goofys](https://github.com/kahing/goofys)). It seems like `-1`/`--max-results=1` doesn't do what it says on the tin.

**Describe what you expected to happen:**

I expected `fd` to print the result and then quit. There is only one file on the path that matches my pattern.

I ran this command: `fd -F --max-results=1 PATTERN PATH`

One result is printed ~5 seconds later, but `fd` continues to run for a long time, at least a minute, until I interrupted it. FWIW, `find` can search the entire path in <25 seconds.

**What version of `fd` are you using?**

```
fd 8.2.1
```

**Which operating system / distribution are you on?**

```
Linux 5.8.0-63-generic x86_64
Distributor ID:	Ubuntu
Description:	Ubuntu 20.10
Release:	20.10
Codename:	groovy
```


Discussion / hints:
Thank you very much. That is definitely not expected. `--max-results` was implemented for this exact reason. Otherwise, we could have gone with `fd … | head -n1`. See https://github.com/sharkdp/fd/pull/555 and #472 for details.

In your case, there might be something else is going on. Could you try to run `fd` with `strace -f` in order to see what `fd` is doing after this long time?
It doesn't really look any different before and after it prints the result, it's just `open`ing directories and `stat`ing VCS directories (which don't exist). I do notice that if I pass `-I` it's quite a bit faster (since it doesn't look for the ignore files). But it still doesn't quit after printing that first result.
Now that I look more closely, soon after printing the result it transitions into a huge string of `nanosleep({tv_sec=0, tv_nsec=1000000})` calls... seems suspicious.
`strace -ftk` on a debug build reveals the sleeps are from here: https://github.com/BurntSushi/ripgrep/blob/master/crates/ignore/src/walk.rs#L1644

Do we need to be telling the walker to quit scanning?
This makes a HUGE difference:

```
diff --git a/src/walk.rs b/src/walk.rs
index 789a500..6cbb175 100644
--- a/src/walk.rs
+++ b/src/walk.rs
@@ -274,6 +274,7 @@ fn spawn_receiver(
                         num_results += 1;
                         if let Some(max_results) = config.max_results {
                             if num_results >= max_results {
+                                wants_to_quit.store(true, Ordering::Relaxed);
                                 break;
                             }
                         }
```

I don't fully understand the purpose of this `wants_to_quit`, but could this be the fix?
I can reproduce this.  I think you're right about the cause, we're still waiting for the worker threads to finish even once the receiver thread is done.
Btw, I was looking into testing this by hacking https://github.com/wfraser/fuse-mt/blob/master/example/src/passthrough.rs to insert a delay before all filesystem operations. More consistent (and cheaper) than hitting AWS over and over. Might be a good way to set up unit tests even.
Thank you for reporting this.

> I can reproduce this. I think you're right about the cause, we're still waiting for the worker threads to finish even once the receiver thread is done.

I don't think we are waiting for the workers threads to completely *finish* the search, but it might be the case that some of them keep running until … well to be honest, I don't know for sure. They will definitely quit once they attempt to send something back over the MPSC channel to the receiver thread (`SendError` / `send_result.is_err()`). But for details we would need to check the `WalkParallel` implementation in `ignore`. There seems to be a `is_quit` shortcut to stop some worker threads.



> I don't fully understand the purpose of this `wants_to_quit`, but could this be the fix?

`wants_to_quit` is only used for `Ctrl-C` detection. We shouldn't reuse it (otherwise we would quit with an error), but we could introduce a second atomic bool for that purpose. I don't really know how to easily reproduce this error (I tried by introducing some `thread::sleep` calls) though.
The key to reproducing this is to search for something that has only one match  For example, I have only one file in my home directory called `title_t00.mkv`, and it is found reasonably early in the search.  But `fd title_t00.mkv ~ -1 -j1` takes a long time to finish.  I assume it's because nothing gets sent from the workers, so they don't see the closed channel.

Fix patch:
diff --git a/src/output.rs b/src/output.rs
index f51d41f63..bcb950501 100644
--- a/src/output.rs
+++ b/src/output.rs
@@ -1,7 +1,6 @@
 use std::borrow::Cow;
 use std::io::{self, Write};
 use std::path::Path;
-use std::sync::atomic::{AtomicBool, Ordering};
 
 use lscolors::{Indicator, LsColors, Style};
 
@@ -15,12 +14,7 @@ fn replace_path_separator(path: &str, new_path_separator: &str) -> String {
 }
 
 // TODO: this function is performance critical and can probably be optimized
-pub fn print_entry<W: Write>(
-    stdout: &mut W,
-    entry: &Path,
-    config: &Config,
-    wants_to_quit: &AtomicBool,
-) {
+pub fn print_entry<W: Write>(stdout: &mut W, entry: &Path, config: &Config) {
     let path = if config.strip_cwd_prefix {
         strip_current_dir(entry)
     } else {
@@ -28,7 +22,7 @@ pub fn print_entry<W: Write>(
     };
 
     let r = if let Some(ref ls_colors) = config.ls_colors {
-        print_entry_colorized(stdout, path, config, ls_colors, wants_to_quit)
+        print_entry_colorized(stdout, path, config, ls_colors)
     } else {
         print_entry_uncolorized(stdout, path, config)
     };
@@ -50,7 +44,6 @@ fn print_entry_colorized<W: Write>(
     path: &Path,
     config: &Config,
     ls_colors: &LsColors,
-    wants_to_quit: &AtomicBool,
 ) -> io::Result<()> {
     // Split the path between the parent and the last component
     let mut offset = 0;
@@ -92,12 +85,6 @@ fn print_entry_colorized<W: Write>(
         writeln!(stdout)?;
     }
 
-    if wants_to_quit.load(Ordering::Relaxed) {
-        // Ignore any errors on flush, because we're about to exit anyway
-        let _ = stdout.flush();
-        ExitCode::KilledBySigint.exit();
-    }
-
     Ok(())
 }
 
diff --git a/src/walk.rs b/src/walk.rs
index 151d5688e..d69c8132a 100644
--- a/src/walk.rs
+++ b/src/walk.rs
@@ -134,11 +134,19 @@ pub fn scan(path_vec: &[PathBuf], pattern: Arc<Regex>, config: Arc<Config>) -> R
 
     let parallel_walker = walker.threads(config.threads).build_parallel();
 
-    let wants_to_quit = Arc::new(AtomicBool::new(false));
+    // Flag for cleanly shutting down the parallel walk
+    let quit_flag = Arc::new(AtomicBool::new(false));
+    // Flag specifically for quitting due to ^C
+    let interrupt_flag = Arc::new(AtomicBool::new(false));
+
     if config.ls_colors.is_some() && config.command.is_none() {
-        let wq = Arc::clone(&wants_to_quit);
+        let quit_flag = Arc::clone(&quit_flag);
+        let interrupt_flag = Arc::clone(&interrupt_flag);
+
         ctrlc::set_handler(move || {
-            if wq.fetch_or(true, Ordering::Relaxed) {
+            quit_flag.store(true, Ordering::Relaxed);
+
+            if interrupt_flag.fetch_or(true, Ordering::Relaxed) {
                 // Ctrl-C has been pressed twice, exit NOW
                 ExitCode::KilledBySigint.exit();
             }
@@ -147,15 +155,15 @@ pub fn scan(path_vec: &[PathBuf], pattern: Arc<Regex>, config: Arc<Config>) -> R
     }
 
     // Spawn the thread that receives all results through the channel.
-    let receiver_thread = spawn_receiver(&config, &wants_to_quit, rx);
+    let receiver_thread = spawn_receiver(&config, &quit_flag, &interrupt_flag, rx);
 
     // Spawn the sender threads.
-    spawn_senders(&config, &wants_to_quit, pattern, parallel_walker, tx);
+    spawn_senders(&config, &quit_flag, pattern, parallel_walker, tx);
 
     // Wait for the receiver thread to print out all results.
     let exit_code = receiver_thread.join().unwrap();
 
-    if wants_to_quit.load(Ordering::Relaxed) {
+    if interrupt_flag.load(Ordering::Relaxed) {
         Ok(ExitCode::KilledBySigint)
     } else {
         Ok(exit_code)
@@ -166,8 +174,10 @@ pub fn scan(path_vec: &[PathBuf], pattern: Arc<Regex>, config: Arc<Config>) -> R
 struct ReceiverBuffer<W> {
     /// The configuration.
     config: Arc<Config>,
+    /// For shutting down the senders.
+    quit_flag: Arc<AtomicBool>,
     /// The ^C notifier.
-    wants_to_quit: Arc<AtomicBool>,
+    interrupt_flag: Arc<AtomicBool>,
     /// Receiver for worker results.
     rx: Receiver<WorkerResult>,
     /// Standard output.
@@ -186,7 +196,8 @@ impl<W: Write> ReceiverBuffer<W> {
     /// Create a new receiver buffer.
     fn new(
         config: Arc<Config>,
-        wants_to_quit: Arc<AtomicBool>,
+        quit_flag: Arc<AtomicBool>,
+        interrupt_flag: Arc<AtomicBool>,
         rx: Receiver<WorkerResult>,
         stdout: W,
     ) -> Self {
@@ -195,7 +206,8 @@ impl<W: Write> ReceiverBuffer<W> {
 
         Self {
             config,
-            wants_to_quit,
+            quit_flag,
+            interrupt_flag,
             rx,
             stdout,
             mode: ReceiverMode::Buffering,
@@ -209,6 +221,7 @@ impl<W: Write> ReceiverBuffer<W> {
     fn process(&mut self) -> ExitCode {
         loop {
             if let Err(ec) = self.poll() {
+                self.quit_flag.store(true, Ordering::Relaxed);
                 return ec;
             }
         }
@@ -244,7 +257,7 @@ impl<W: Write> ReceiverBuffer<W> {
                         }
                     }
                     ReceiverMode::Streaming => {
-                        self.print(&path);
+                        self.print(&path)?;
                         self.flush()?;
                     }
                 }
@@ -273,8 +286,16 @@ impl<W: Write> ReceiverBuffer<W> {
     }
 
     /// Output a path.
-    fn print(&mut self, path: &Path) {
-        output::print_entry(&mut self.stdout, path, &self.config, &self.wants_to_quit)
+    fn print(&mut self, path: &Path) -> Result<(), ExitCode> {
+        output::print_entry(&mut self.stdout, path, &self.config);
+
+        if self.interrupt_flag.load(Ordering::Relaxed) {
+            // Ignore any errors on flush, because we're about to exit anyway
+            let _ = self.flush();
+            return Err(ExitCode::KilledBySigint);
+        }
+
+        Ok(())
     }
 
     /// Switch ourselves into streaming mode.
@@ -283,7 +304,7 @@ impl<W: Write> ReceiverBuffer<W> {
 
         let buffer = mem::take(&mut self.buffer);
         for path in buffer {
-            self.print(&path);
+            self.print(&path)?;
         }
 
         self.flush()
@@ -315,11 +336,13 @@ impl<W: Write> ReceiverBuffer<W> {
 
 fn spawn_receiver(
     config: &Arc<Config>,
-    wants_to_quit: &Arc<AtomicBool>,
+    quit_flag: &Arc<AtomicBool>,
+    interrupt_flag: &Arc<AtomicBool>,
     rx: Receiver<WorkerResult>,
 ) -> thread::JoinHandle<ExitCode> {
     let config = Arc::clone(config);
-    let wants_to_quit = Arc::clone(wants_to_quit);
+    let quit_flag = Arc::clone(quit_flag);
+    let interrupt_flag = Arc::clone(interrupt_flag);
 
     let show_filesystem_errors = config.show_filesystem_errors;
     let threads = config.threads;
@@ -375,7 +398,7 @@ fn spawn_receiver(
             let stdout = stdout.lock();
             let stdout = io::BufWriter::new(stdout);
 
-            let mut rxbuffer = ReceiverBuffer::new(config, wants_to_quit, rx, stdout);
+            let mut rxbuffer = ReceiverBuffer::new(config, quit_flag, interrupt_flag, rx, stdout);
             rxbuffer.process()
         }
     })
@@ -439,7 +462,7 @@ impl DirEntry {
 
 fn spawn_senders(
     config: &Arc<Config>,
-    wants_to_quit: &Arc<AtomicBool>,
+    quit_flag: &Arc<AtomicBool>,
     pattern: Arc<Regex>,
     parallel_walker: ignore::WalkParallel,
     tx: Sender<WorkerResult>,
@@ -448,10 +471,10 @@ fn spawn_senders(
         let config = Arc::clone(config);
         let pattern = Arc::clone(&pattern);
         let tx_thread = tx.clone();
-        let wants_to_quit = Arc::clone(wants_to_quit);
+        let quit_flag = Arc::clone(quit_flag);
 
         Box::new(move |entry_o| {
-            if wants_to_quit.load(Ordering::Relaxed) {
+            if quit_flag.load(Ordering::Relaxed) {
                 return ignore::WalkState::Quit;
             }
 


Test patch: