bootandy/archived-dust
1
0
Fork 0
mirror of https://github.com/bootandy/dust.git synced 2026-06-08 11:29:05 +03:00
Code Issues 21 Packages Projects Releases 10 Wiki Activity

[GH-ISSUE #477] ^C fails to immediately abort indexing #209

New Issue
Closed
opened 2026-06-08 11:26:08 +03:00 by zhus · 2 comments
zhus commented 2026-06-08 11:26:08 +03:00
Owner
Copy Link
Copy Source

Originally created by @quantatic on GitHub (Mar 14, 2025).
Original GitHub issue: https://github.com/bootandy/dust/issues/477

This is an issue very similar to https://github.com/bootandy/dust/issues/367, though I'm opening a separate issue because the issue manifests in a slightly different way.

Expected Behavior

When I send a SIGINT signal to a running dust process (via kill or C-c), I expect the process to quit immedietly.

Actual Behavior

When indexing a large directory, or indexing on a slow filesystem, attempting to interrupt the process causes dust to continue processing, sometimes for a long period of time. I am able to kill it via SIGKILL, though intuitively, I might prefer it to handle a C-c in a way that immediately kills the process.

Example Reproduction

Indexing: ../my_directory 1596252 files, 6.6T ... \^C
Aborting
Indexing: ../my_directory 1604186 files, 6.6T ... /^C
Aborting
Indexing: ../my_directory 1672889 files, 6.6T ... -^C
Aborting
Indexing: ../my_directory 1947295 files, 6.7T ... |^C
Aborting
Indexing: ../my_directory 2415938 files, 6.9T ... \Killed
<process exits>

In this reproduction, I attempt to exit the running process with C-c several times, which lets me know that the process is "Aborting", but continues running. Finally, I kill the process with kill -9 <PID>, at which point the process exits.

Investigation

I am able to consistently reproduce this issue on the most recent commit as of the posting of this issue: dd799706fbd062b468d5e8e219c0ef332344588b.

It seems that the SIGINT handler configured attempts to perform a graceful shutdown when indexing is in process, choosing to set error_listen_for_ctrlc.abort rather than immediately exiting.

This flag is checked during directory walking, but only at the very beginning of a walk for a directory -- if a walk for a large directory (or a directory on a very slow filesystem) is already in progress, it will wait until the walk completes, which may take a significant period of time.

Suggestions

I'm not entirely certain why we set error_listen_for_ctrlc.abort when we receive SIGINT, rather than simply exiting the process immediately. Unless there's some behavior I'm not aware of we're attempting to handle, it seems to me that we could rewrite this function as follows:

diff --git a/src/main.rs b/src/main.rs
index 0a976eb..79def97 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -126,9 +126,7 @@ fn main() {
     ctrlc::set_handler(move || {
         error_listen_for_ctrlc.lock().unwrap().abort = true;
         println!("\nAborting");
-        if cloned_is_in_listing.load(Ordering::Relaxed) {
-            process::exit(1);
-        }
+        process::exit(1);
     })
     .expect("Error setting Ctrl-C handler");

When running with this patch applied, I get the behavior I expect -- namely, an exit as soon as SIGINT is received:

Indexing: ../my_directory 1140107 files, 6.4T ... -^C
Aborting
<process exits>

If we do want to handle SIGINT gracefully for some reason I'm not aware of, I might suggest some sort of flag that forcefully exits the process on the second SIGINT, while displaying a message during the first that we are attempting to shut down gracefully and to press C-c for immediate abort (like the following, approximately):

Indexing: ../my_directory 1596252 files, 6.6T ... \^C
Aborting indexing, press C-c again to force immediate abort
Indexing: ../my_directory 1740107 files, 7.4T ... -^C
Forcefully aborting now
<process exits>

Alternatively, some sort of command-line option that immediately aborts rather than attempting to finalize indexing may be a viable option.

I'm happy to put out a PR for any of these changes (though in my personal opinion, the first approach -- immediate exit -- meshes best with how I would naively expect the program to behave).

Thanks for the great tool! I've gotten a lot of great usage out of it 😄

Originally created by @quantatic on GitHub (Mar 14, 2025). Original GitHub issue: https://github.com/bootandy/dust/issues/477 This is an issue very similar to https://github.com/bootandy/dust/issues/367, though I'm opening a separate issue because the issue manifests in a slightly different way. ### Expected Behavior When I send a SIGINT signal to a running `dust` process (via `kill` or `C-c`), I expect the process to quit immedietly. ### Actual Behavior When indexing a large directory, or indexing on a slow filesystem, attempting to interrupt the process causes `dust` to continue processing, sometimes for a long period of time. I am able to kill it via SIGKILL, though intuitively, I might prefer it to handle a `C-c` in a way that immediately kills the process. #### Example Reproduction ``` Indexing: ../my_directory 1596252 files, 6.6T ... \^C Aborting Indexing: ../my_directory 1604186 files, 6.6T ... /^C Aborting Indexing: ../my_directory 1672889 files, 6.6T ... -^C Aborting Indexing: ../my_directory 1947295 files, 6.7T ... |^C Aborting Indexing: ../my_directory 2415938 files, 6.9T ... \Killed <process exits> ``` In this reproduction, I attempt to exit the running process with `C-c` several times, which lets me know that the process is "Aborting", but continues running. Finally, I kill the process with `kill -9 <PID>`, at which point the process exits. ### Investigation I am able to consistently reproduce this issue on the most recent commit as of the posting of this issue: `dd799706fbd062b468d5e8e219c0ef332344588b`. It seems that the [SIGINT](https://github.com/bootandy/dust/blob/dd799706fbd062b468d5e8e219c0ef332344588b/src/main.rs#L129) handler configured attempts to perform a graceful shutdown when indexing is in process, choosing to set `error_listen_for_ctrlc.abort` rather than immediately exiting. This flag is checked during [directory walking](https://github.com/bootandy/dust/blob/dd799706fbd062b468d5e8e219c0ef332344588b/src/dir_walker.rs#L209), but only at the very beginning of a walk for a directory -- if a walk for a large directory (or a directory on a very slow filesystem) is already in progress, it will wait until the walk completes, which may take a significant period of time. ### Suggestions I'm not entirely certain why we set `error_listen_for_ctrlc.abort` when we receive SIGINT, rather than simply exiting the process immediately. Unless there's some behavior I'm not aware of we're attempting to handle, it seems to me that we could rewrite this function as follows: ``` diff --git a/src/main.rs b/src/main.rs index 0a976eb..79def97 100644 --- a/src/main.rs +++ b/src/main.rs @@ -126,9 +126,7 @@ fn main() { ctrlc::set_handler(move || { error_listen_for_ctrlc.lock().unwrap().abort = true; println!("\nAborting"); - if cloned_is_in_listing.load(Ordering::Relaxed) { - process::exit(1); - } + process::exit(1); }) .expect("Error setting Ctrl-C handler"); ``` When running with this patch applied, I get the behavior I expect -- namely, an exit as soon as SIGINT is received: ``` Indexing: ../my_directory 1140107 files, 6.4T ... -^C Aborting <process exits> ``` If we _do_ want to handle SIGINT gracefully for some reason I'm not aware of, I might suggest some sort of flag that forcefully exits the process on the second SIGINT, while displaying a message during the first that we are attempting to shut down gracefully and to press `C-c` for immediate abort (like the following, approximately): ``` Indexing: ../my_directory 1596252 files, 6.6T ... \^C Aborting indexing, press C-c again to force immediate abort Indexing: ../my_directory 1740107 files, 7.4T ... -^C Forcefully aborting now <process exits> ``` Alternatively, some sort of command-line option that immediately aborts rather than attempting to finalize indexing may be a viable option. I'm happy to put out a PR for any of these changes (though in my personal opinion, the first approach -- immediate exit -- meshes best with how I would naively expect the program to behave). Thanks for the great tool! I've gotten a lot of great usage out of it 😄
zhus closed this issue 2026-06-08 11:26:08 +03:00
zhus commented 2026-06-08 11:26:09 +03:00
Author
Owner
Copy Link
Copy Source

@bootandy commented on GitHub (Mar 15, 2025):

I'm not entirely certain why we set error_listen_for_ctrlc.abort when we receive SIGINT, rather than simply exiting the process immediately. Unless there's some behavior I'm not aware of we're attempting to handle, it seems to me that we

Honestly, I was simply trying to do a clean shutdown.

if a walk for a large directory (or a directory on a very slow filesystem) is already in progress, it will wait until the walk completes, which may take a significant period of time.

I was aware that if it scanned a large directory it would take a while to exit but didn't think it would be a problem. If its an issue we can fix it.

could rewrite this function as follows:

I'm trying to remember why I didn't do that in the first place ..... and I can't. I think it just felt cleaner at the time.

In summary: I think - Yes we'll try your simple patch, if you want to make a PR I'll accept. As you say I think the first approach is probably best - if there are problems with that we'll figure that out later.

<!-- gh-comment-id:2725825295 --> @bootandy commented on GitHub (Mar 15, 2025): >I'm not entirely certain why we set error_listen_for_ctrlc.abort when we receive SIGINT, rather than simply exiting the process immediately. Unless there's some behavior I'm not aware of we're attempting to handle, it seems to me that we Honestly, I was simply trying to do a clean shutdown. > if a walk for a large directory (or a directory on a very slow filesystem) is already in progress, it will wait until the walk completes, which may take a significant period of time. I was aware that if it scanned a large directory it would take a while to exit but didn't think it would be a problem. If its an issue we can fix it. >could rewrite this function as follows: I'm trying to remember why I didn't do that in the first place ..... and I can't. I think it just felt cleaner at the time. In summary: I think - Yes we'll try your simple patch, if you want to make a PR I'll accept. As you say I think the first approach is probably best - if there are problems with that we'll figure that out later.
zhus commented 2026-06-08 11:26:09 +03:00
Author
Owner
Copy Link
Copy Source

@bootandy commented on GitHub (Mar 15, 2025):

Also I'm on vacation next week (ski!). So I'm going to go quite for at least a week.

Thanks,

<!-- gh-comment-id:2725825845 --> @bootandy commented on GitHub (Mar 15, 2025): Also I'm on vacation next week (ski!). So I'm going to go quite for at least a week. Thanks,
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
good first issue
help wanted
pull-request
Mirrored from GitHub Pull Request
 windows help wanted
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
zhus (Sergey Zhukov)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: bootandy/archived-dust#209
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?