Repository navigation
linux: the wrap walks its patterns in turns, and stops when its signal is aborted - #628
Merged
Merged
Conversation
A denyRead pattern with `**` is expanded by listing everything under its base, at every command and on the caller's thread. Under a large directory that holds the caller up for seconds, once per pattern. For every entry the walk spelled two paths and converted one before it asked whether the entry matched. A pattern that splits into path components is matched by name, so a plain file it does not match, which is nearly every entry of a large tree and which nothing further on records, now costs no path at all. The walk already kept each directory's listing for a second position to read. The patterns of one read configuration now share that map, so those with a base in common list each directory once between them. The map lives for one expansion: the next command lists afresh. Six patterns over 400,000 files in 20,000 directories: one wrap 6.2 s before, 0.97 s after; resident memory 469 MB before, 286 MB after; the same mounts.
…l is aborted Expanding a `denyRead` or `allowRead` pattern lists everything under its base. That ran from start to finish on the caller's thread: for as long as it took, the caller could neither draw nor hear that it should stop, and the AbortSignal that wrapWithSandbox() takes could not fire. The walk is now a generator, a step to a directory, and so are the two functions between it and the wrap: one implementation, run in one of two ways. getFsReadConfig(), which is synchronous by signature, runs it on the spot. wrapWithSandbox() lets the event loop have a turn every 10 ms and rejects with the signal's reason once that is aborted. The wrap reads every filesystem setting before the first turn, so a configuration replaced meanwhile does not reach it, as before. The CLI passed no signal, and its own handlers are installed once there is a child to pass an interrupt on to. Heard during the walk, an interrupt was therefore taken by the library's clean-up alone and the command ran all the same. It now stops the wrap: exit 1, at once, nothing run. Six patterns over 400,000 files: the event loop held for 950 ms at a stretch before, 13 to 18 ms after; one wrap 0.98 s before, 1.0 s after; the same mounts.
…borted one hands nothing out Two faults in the change before this one, and one it brought to light. With the allowRead patterns walked in turns too, the denyRead list and the network flags were read after the first turn. updateConfig() can run in a turn, so a wrap could get the writes of one configuration and the reads of another: a file the first denied was readable. Everything the configuration says about files and about restricting the network is again read in one stretch, ahead of the first turn, as before the walk was in turns. The scan for mandatory denies below the working directory takes the wrap's signal, and swallowed every failure of ripgrep as "nothing found", an abort included. An aborted wrap then resolved with a command that lacked those denies. It now rejects. The CLI has passed a signal since the last change, so an interrupt during the scan reached that path. finishInTurns() looks at the signal once more before it returns, and the CLI looks at its own after the wrap and before it spawns.
…om the start The last commit read the file lists and the network flags ahead of the first turn and said that the configuration was read there. It was not: some twenty settings are read after the last turn (allowGitConfig, the search depth, the unix sockets, the weaker nested sandbox). On main they follow an await that has already resolved, so nothing comes between. With the walk in turns updateConfig() can, and a command could get the write list of one configuration with the allowGitConfig of another: a policy nobody chose. The wrap now remembers the configuration it started with and, if that has been replaced by the time its last await is over, starts over. Nothing from there to the platform's wrapper awaits, so every setting is read from one configuration, and the command is the one an undisturbed wrap under the new configuration gives. The reads are back in their original order. The CLI installed its interrupt handlers just ahead of the wrap. initialize() installs the library's own, which only clean up, so an interrupt between the two was taken by those alone and the command was run all the same, as on main. The CLI's are now installed first.
ronleizrowice-ant
added a commit
that referenced
this pull request
Sep 30, 2026
The walk's budget (#607), its shared listings (#627) and its steps (#628) rewrite one code path, and the path entries of #620 to #622 sit on top of it. Everything above the walk is a generator now: the read-deny expansion, the function readDenyGlobExpander() returns, the allowRead expansion, the resolution of read path entries, and withOtherReadings(), which calls the expander once for each reading of an entry. The synchronous names finish those on the spot. In the walk the step comes first and the budget's clock is read after it, before the next listing. The deadline is wall time, so turns given to other work count against it: a wrap can be refused earlier for that, and a list is never cut short. The budget error is given its code around the delegated generator. An abort is raised by the driver and does not pass through there, so a wrap with a spent budget and an aborted signal rejects with the signal's reason. A wrap that starts over makes a new budget and new listings, and reads the new configuration's denyReadGlobBudget. In the scan's catch the abort is looked at first, then what ripgrep listed.
antdres
previously approved these changes
Sep 30, 2026
ronleizrowice-ant
dismissed
antdres’s stale review
September 30, 2026 12:23
The base branch was changed.
The change this one was stacked on was merged as one commit, so its lines conflicted with the same lines here. This branch's side is taken in all seven places: it is that change with the steps on top. src/ and test/ are as they were before the merge.
antdres
approved these changes
Sep 30, 2026
ronleizrowice-ant
enabled auto-merge (squash)
September 30, 2026 12:28
This was referenced Oct 1, 2026
Merged
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.
Follows #627, which is merged. Expanding a
denyReadorallowReadpattern lists everything under its base, from start to finish on the caller's thread. For as long as that took the caller could neither draw nor hear that it should stop, and theAbortSignalthatwrapWithSandbox()takes could not fire. Now the thread is given back every 10 ms and the signal works.What changes
getFsReadConfig(), synchronous by signature, runs it on the spot (finish).wrapWithSandbox()runs it in turns and rejects with the signal's reason once aborted (finishInTurns).updateConfig()can run in a turn, and some twenty settings are read after the last one. The wrap remembers the configuration it started with and, if that has been replaced once its last await is over, starts over. Nothing from there to the platform's wrapper awaits. The command is the one an undisturbed wrap under the new configuration gives.mainan aborted wrap resolves with a command that lacks those denies. It now rejects.initialize()installs, only clean up: onmainan interrupt during start-up or the wrap is swallowed, and the command can run all the same. It now stops the run: exit 1, nothing run.Measured
Six patterns over 400,000 files:
The mounts are the same, and nothing more is held after a wrap.
Known limits
getFsReadConfig()still holds the thread. One listing of one directory is not cut short, and the work after the walk is not in turns.Testing
main: identical results over an awkward tree, and withupdateConfig(),reset()or a second wrap during a wrap.