Conversation
Reading extended attributes is a two call protocol: one call with no buffer reports the size, a second call fills a buffer of that size. When the size turned out to be too small the second call fails with ERANGE and the read was retried. That retry had no limit, so a file system that keeps answering with a size its own second call rejects loops forever and eza never finishes listing the directory. Limit the retries and let the error through when they run out. The caller already treats a failed lookup as having no attributes, so a mount that cannot be queried now costs one log line instead of a hang. Retries that do converge are unaffected. Adds tests covering a mount that refuses every buffer, a size that grows once, and an error that a larger buffer cannot fix.
cargo fmt wanted the call count in a local before the assertion, so spell out what is being counted rather than leaving a long expression inline.
The tests set the errno slot directly to drive a specific error path, and that slot is spelled differently per platform: __error on macOS, iOS and FreeBSD, __errno_location on Linux, and netbsd and openbsd expose neither. Choosing __errno_location for everything that was not macOS or iOS therefore failed to compile on FreeBSD. Use the symbol each platform actually has, and leave the tests out on the two platforms that expose no way to reach the slot.
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.
Closes #1850.
The hang
Reading extended attributes is a two call protocol. The first call passes no buffer and gets back the size, the second passes a buffer of that size and gets the data back. When the second call fails with
ERANGEthe buffer was too small, so the read is retried against a freshly asked for size.That retry had no limit. A file system that keeps reporting a size its own second call then rejects never lets the loop finish, and since this runs per entry while listing,
ezanever finishes listing the directory. Thefs_usagetrace in the report shows exactly this:listxattragainst the mount alternating between a size andERANGEfor as long as the capture runs.The fix
Bound the retries at
MAX_ERANGE_RETRIES = 5and return the error once they are used up. A mount that cannot be queried now costs one log line and a listing that completes, instead of a listing that never does.Retries that do converge are unaffected, so a size that grows once between the two calls is still picked up. Errors a larger buffer cannot fix are still reported immediately rather than retried.
File::gather_extended_attributesalready logs a failed lookup and carries on with no attributes, so the error path needed no change.Tests
Three tests drive
get_loopwith a stand-in for the file system:ERANGEafter a bounded number of callsThe first test does not pass against the previous code, where it spins instead of returning. Full suite: 358 passed, 0 failed.