Skip to content

ls: prevent ReadDir from closing before entries are processed - #9410

Merged
sylvestre merged 1 commit into
uutils:mainfrom
vikram-kangotra:fix/ls-proc-self-fd-regression
Nov 26, 2025
Merged

ls: prevent ReadDir from closing before entries are processed#9410
sylvestre merged 1 commit into
uutils:mainfrom
vikram-kangotra:fix/ls-proc-self-fd-regression

Conversation

@vikram-kangotra

Copy link
Copy Markdown
Contributor

The issue occurred because ReadDir was being consumed by the for loop, causing it to be dropped immediately after iteration. This closed the directory file descriptor (FD 3) before metadata() could be called on the entries, since /proc/self/fd shows the process's own open FDs.

By using read_dir.by_ref() in the loop, we borrow the iterator instead of consuming it, keeping the ReadDir (and its underlying FD) alive until enter_directory() completes.

Fixes: #9332

@github-actions

Copy link
Copy Markdown

GNU testsuite comparison:

Congrats! The gnu test tests/fold/fold is no longer failing!

@vikram-kangotra
vikram-kangotra force-pushed the fix/ls-proc-self-fd-regression branch from 08208a9 to d469b5c Compare November 21, 2025 04:38
@codspeed-hq

codspeed-hq Bot commented Nov 21, 2025

Copy link
Copy Markdown

CodSpeed Performance Report

Merging #9410 will degrade performances by 2.16%

Comparing vikram-kangotra:fix/ls-proc-self-fd-regression (93c22fd) with main (909553a)

Summary

⚡ 1 improvement
❌ 1 regression
✅ 124 untouched
⏩ 6 skipped1

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

Benchmark BASE HEAD Change
ls_recursive_long_all_deep_tree[(100, 4)] 3.2 ms 3.3 ms -2.16%
ls_recursive_wide_tree[(10000, 1000)] 52.6 ms 51.4 ms +2.44%

Footnotes

  1. 6 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@github-actions

Copy link
Copy Markdown

GNU testsuite comparison:

Skip an intermittent issue tests/misc/tee (fails in this run but passes in the 'main' branch)

@sylvestre

Copy link
Copy Markdown
Contributor

Would it be possible to add a test to make sure we don't regress in the future? Thanks

@vikram-kangotra

Copy link
Copy Markdown
Contributor Author

Would it be possible to add a test to make sure we don't regress in the future? Thanks

Sure. Will add test

@vikram-kangotra
vikram-kangotra force-pushed the fix/ls-proc-self-fd-regression branch from d469b5c to 83b6f88 Compare November 21, 2025 17:59
@sylvestre

Copy link
Copy Markdown
Contributor

Some jobs are failing

@github-actions

Copy link
Copy Markdown

GNU testsuite comparison:

Skipping an intermittent issue tests/tail/overlay-headers (passes in this run but fails in the 'main' branch)

The issue occurred because ReadDir was being consumed by the for loop,
causing it to be dropped immediately after iteration. This closed the
directory file descriptor (FD 3) before metadata() could be called on
the entries, since /proc/self/fd shows the process's own open FDs.

By using read_dir.by_ref() in the loop, we borrow the iterator instead
of consuming it, keeping the ReadDir (and its underlying FD) alive until
enter_directory() completes.

Fixes: uutils#9332
@vikram-kangotra
vikram-kangotra force-pushed the fix/ls-proc-self-fd-regression branch from 83b6f88 to 93c22fd Compare November 21, 2025 18:28
@github-actions

Copy link
Copy Markdown

GNU testsuite comparison:

Skip an intermittent issue tests/misc/tee (fails in this run but passes in the 'main' branch)

@vikram-kangotra

Copy link
Copy Markdown
Contributor Author

@sylvestre I have fixed the failing jobs, but the Performance Analysis is failing. Would that be an issue. Could you please look into it?

@sylvestre
sylvestre merged commit df959b7 into uutils:main Nov 26, 2025
126 of 127 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

0.3.0 regression: ls: cannot access '/proc/self/fd/3': No such file or directory

2 participants