fs: Use readdir() instead of readdir_r() on Linux and Android#92778

tavianator · 2022-01-11T19:16:13Z

See #40021 for more details. Fixes #86649. Fixes #34668.

rust-highfive · 2022-01-11T19:16:16Z

r? @joshtriplett

(rust-highfive has picked a reviewer for you, use r? to override)

joshtriplett · 2022-01-11T19:38:32Z

@bors r+ rollup

bors · 2022-01-11T19:38:34Z

📌 Commit ea9181b has been approved by joshtriplett

tavianator · 2022-01-11T20:00:27Z

Hang on, there's a bug in metadata()

…shtriplett fs: Use readdir() instead of readdir_r() on Linux readdir() is preferred over readdir_r() on Linux and many other platforms because it more gracefully supports long file names. Both glibc and musl (and presumably all other Linux libc implementations) guarantee that readdir() is thread-safe as long as a single DIR* is not accessed concurrently, which is enough to make a readdir()-based implementation of ReadDir safe. This implementation is already used for some other OSes including Fuchsia, Redox, and Solaris. See rust-lang#40021 for more details. Fixes rust-lang#86649.

matthiaskrgr · 2022-01-11T20:28:28Z

@bors r-

tavianator · 2022-01-11T20:42:54Z

Okay, should be fixed.

r? @joshtriplett

joshtriplett · 2022-01-11T21:25:37Z

@bors r+

bors · 2022-01-11T21:25:38Z

📌 Commit 4839b81 has been approved by joshtriplett

…shtriplett fs: Use readdir() instead of readdir_r() on Linux readdir() is preferred over readdir_r() on Linux and many other platforms because it more gracefully supports long file names. Both glibc and musl (and presumably all other Linux libc implementations) guarantee that readdir() is thread-safe as long as a single DIR* is not accessed concurrently, which is enough to make a readdir()-based implementation of ReadDir safe. This implementation is already used for some other OSes including Fuchsia, Redox, and Solaris. See rust-lang#40021 for more details. Fixes rust-lang#86649.

ehuss · 2022-01-14T03:36:05Z

@bors r-

Failed in rollup: #92878 (comment)

tavianator · 2022-01-14T19:44:18Z

I switched Android over too, and split up the commits a bit. I would test the Android build locally but I have no idea how. Is there documentation for building Rust for Android or should I just try to follow the CI script?

r? @joshtriplett

ehuss · 2022-01-14T21:34:46Z

To test android, it needs the NDK. There is a script here for getting it, but I would just use the docker image. That can be done with src/ci/docker/run.sh dist-android. There is documentation at https://rustc-dev-guide.rust-lang.org/tests/intro.html#testing-with-docker-images for testing with docker.

tavianator · 2022-01-14T22:00:49Z

Thanks! I ran the library tests in that Docker image and they passed.

bors · 2022-01-20T23:52:52Z

☔ The latest upstream changes (presumably #93119) made this pull request unmergeable. Please resolve the merge conflicts.

readdir() is preferred over readdir_r() on Linux and many other platforms because it more gracefully supports long file names. Both glibc and musl (and presumably all other Linux libc implementations) guarantee that readdir() is thread-safe as long as a single DIR* is not accessed concurrently, which is enough to make a readdir()-based implementation of ReadDir safe. This implementation is already used for some other OSes including Fuchsia, Redox, and Solaris. See rust-lang#40021 for more details. Fixes rust-lang#86649. Fixes rust-lang#34668.

Bionic also guarantees that readdir() is thread-safe enough.

tavianator · 2022-01-21T15:17:41Z

Rebased! Linux and Android tests still pass

tavianator · 2022-01-26T15:51:23Z

@rustbot label -S-waiting-on-author +S-waiting-on-review

joshtriplett · 2022-01-26T19:24:53Z

@bors r+ rollup

bors · 2022-01-26T19:24:54Z

📌 Commit 3eeb3ca has been approved by joshtriplett

…askrgr Rollup of 7 pull requests Successful merges: - rust-lang#90247 (Improve Duration::try_from_secs_f32/64 accuracy by directly processing exponent and mantissa) - rust-lang#91861 (Replace iterator-based construction of collections by `Into<T>`) - rust-lang#92098 (add OpenBSD platform-support page) - rust-lang#92134 (Add x86_64-pc-windows-msvc linker-plugin-lto instructions) - rust-lang#92256 (Improve selection errors for `~const` trait bounds) - rust-lang#92778 (fs: Use readdir() instead of readdir_r() on Linux and Android) - rust-lang#93338 (Update minifier crate version to 0.0.42) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

Bug rust-lang#50619 was fixed by adding an end_of_stream flag in rust-lang#50630. Unfortunately, that fix only applied to the readdir_r() path. When I switched Linux to use readdir() in rust-lang#92778, I inadvertently reintroduced the bug on that platform. Other platforms that had always used readdir() were presumably never fixed. This patch enables end_of_stream for all platforms, and adds a Linux-specific regression test that should hopefully prevent the bug from being reintroduced again.

…imulacrum fs: Fix rust-lang#50619 (again) and add a regression test Bug rust-lang#50619 was fixed by adding an end_of_stream flag in rust-lang#50630. Unfortunately, that fix only applied to the readdir_r() path. When I switched Linux to use readdir() in rust-lang#92778, I inadvertently reintroduced the bug on that platform. Other platforms that had always used readdir() were presumably never fixed. This patch enables end_of_stream for all platforms, and adds a Linux-specific regression test that should hopefully prevent the bug from being reintroduced again.

rust-highfive assigned joshtriplett Jan 11, 2022

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 11, 2022

tavianator force-pushed the linux-readdir-no-r branch from 0c5acfb to ea9181b Compare January 11, 2022 19:20

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 11, 2022

tavianator marked this pull request as draft January 11, 2022 20:19

matthiaskrgr mentioned this pull request Jan 11, 2022

Rollup of 16 pull requests #92784

Closed

bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jan 11, 2022

tavianator force-pushed the linux-readdir-no-r branch from ea9181b to 4839b81 Compare January 11, 2022 20:40

tavianator marked this pull request as ready for review January 11, 2022 20:40

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jan 11, 2022

matthiaskrgr mentioned this pull request Jan 14, 2022

Rollup of 13 pull requests #92878

Closed

bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jan 14, 2022

tavianator force-pushed the linux-readdir-no-r branch from 4839b81 to 64c21ae Compare January 14, 2022 19:24

tavianator changed the title ~~fs: Use readdir() instead of readdir_r() on Linux~~ fs: Use readdir() instead of readdir_r() on Linux and Android Jan 14, 2022

tavianator added 3 commits January 21, 2022 07:59

fs: Implement more ReadDir methods in terms of name_cstr()

c3e92fe

fs: Use readdir() instead of readdir_r() on Android

3eeb3ca

Bionic also guarantees that readdir() is thread-safe enough.

tavianator force-pushed the linux-readdir-no-r branch from 64c21ae to 3eeb3ca Compare January 21, 2022 13:04

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jan 26, 2022

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 26, 2022

matthiaskrgr mentioned this pull request Jan 26, 2022

Rollup of 7 pull requests #93352

Merged

bors merged commit 253f64c into rust-lang:master Jan 27, 2022

rustbot added this to the 1.60.0 milestone Jan 27, 2022

tavianator deleted the linux-readdir-no-r branch January 27, 2022 20:24

tmiasko mentioned this pull request Jan 29, 2022

compiletest crashes on CI with: expected success, got: signal: 11 (core dumped) #93384

Closed

This was referenced Dec 12, 2022

fd can get stuck when ran onto the whole FS from the root sharkdp/fd#288

Closed

fs: Fix #50619 (again) and add a regression test #105638

Merged

Uh oh!

Conversation

tavianator commented Jan 11, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rust-highfive commented Jan 11, 2022

Uh oh!

joshtriplett commented Jan 11, 2022

Uh oh!

bors commented Jan 11, 2022

Uh oh!

tavianator commented Jan 11, 2022

Uh oh!

matthiaskrgr commented Jan 11, 2022

Uh oh!

tavianator commented Jan 11, 2022

Uh oh!

joshtriplett commented Jan 11, 2022

Uh oh!

bors commented Jan 11, 2022

Uh oh!

ehuss commented Jan 14, 2022

Uh oh!

tavianator commented Jan 14, 2022

Uh oh!

ehuss commented Jan 14, 2022

Uh oh!

tavianator commented Jan 14, 2022

Uh oh!

bors commented Jan 20, 2022

Uh oh!

tavianator commented Jan 21, 2022

Uh oh!

tavianator commented Jan 26, 2022

Uh oh!

joshtriplett commented Jan 26, 2022

Uh oh!

bors commented Jan 26, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

tavianator commented Jan 11, 2022 •

edited

Loading