Repository navigation
fix: call recursive_delete/recursive_copy callback only once - #788
Merged
stevearc merged 1 commit intoOct 12, 2026
Merged
Conversation
Owner
|
LGTM, thanks! |
This was referenced Oct 12, 2026
Contributor
Author
|
Thanks for merging! With this fix a big delete no longer crashes, but it can still fail partway with EMFILE when the open file limit is 1024 (the systemd default). I have a version on my fork that reads each directory fully and closes it before going deeper, with a cap on how many are open at once. It deletes a 300 MB |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Deleting a large directory such as
node_modulesfrom oil can abort Neovim withfree(): double free detected in tcache 2. This is the crash reported in #719, andprobably the one in #761.
recursive_deleteremoves all entries of a directory batch at the same time. With thedefault soft limit of 1024 open files, a tree with a few thousand directories makes
fs_opendirfail withEMFILEfor many of them at once. The first error reachescomplete, which replaces itself with a no-op and callsinner_cb. Replacing it onlychanges the local variable: each child operation already holds the original function, so
every later error calls
inner_cbagain, and each call runsuv.fs_closedir(fd)on thesame handle.
The no-op reassignment never worked, but before #712 it was unreachable:
completechecked
errinstead oferr2, so child errors were ignored.recursive_copyuses thesame pattern. It doesn't crash because it never closes the directory, though it still
calls its callback more than once.
This change adds a
failedflag shared by all reads of the directory, soinner_cbrunsonce and later errors are ignored.
Reproduction
Two failures in the same directory are enough, so permissions give a simpler repro than
the file limit:
Delete
repro/in oil and save. Without this change Neovim aborts; with it, oil shows asingle
EACCESerror and Neovim keeps running. A single failing entry was alreadyreported correctly.
With
ulimit -Sn 1024, deleting a bignode_modulesnow fails with oneEMFILEerrorinstead of crashing. Oil still can't remove the whole tree in one pass, because it opens
directories with no concurrency limit.
Fixes #719