-
Notifications
You must be signed in to change notification settings - Fork 13.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fs: Fix #50619 (again) and add a regression test #105638
Conversation
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.
(rustbot has picked a reviewer for you, use r? to override) |
Hey! It looks like you've submitted a new PR for the library teams! If this PR contains changes to any Examples of
|
A comment on the original fix said:
Which I took as a challenge :) The test I added creates a zombie child process and uses
I could not make it spuriously fail even running it 4,096 times in parallel. |
@bors r+ rollup=iffy Thanks! |
☀️ Test successful - checks-actions |
Finished benchmarking commit (48b3c46): comparison URL. Overall result: ✅ improvements - no action needed@rustbot label: -perf-regression Instruction countThis is a highly reliable metric that was used to determine the overall result at the top of this comment.
Max RSS (memory usage)ResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesThis benchmark run did not return any relevant results for this metric. |
…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.
Bug #50619 was fixed by adding an end_of_stream flag in #50630.
Unfortunately, that fix only applied to the readdir_r() path. When I
switched Linux to use readdir() in #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.