Skip to content

Commit 48b3c46

Browse files
committed
Auto merge of rust-lang#105638 - tavianator:fix-50619-again, r=Mark-Simulacrum
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.
2 parents ff016a5 + 9fb7c5a commit 48b3c46

File tree

2 files changed

+47
-38
lines changed

2 files changed

+47
-38
lines changed

library/std/src/fs/tests.rs

+28
Original file line numberDiff line numberDiff line change
@@ -1567,3 +1567,31 @@ fn test_eq_direntry_metadata() {
15671567
assert_eq!(ft1, ft2);
15681568
}
15691569
}
1570+
1571+
/// Regression test for https://github.com/rust-lang/rust/issues/50619.
1572+
#[test]
1573+
#[cfg(target_os = "linux")]
1574+
fn test_read_dir_infinite_loop() {
1575+
use crate::io::ErrorKind;
1576+
use crate::process::Command;
1577+
1578+
// Create a zombie child process
1579+
let Ok(mut child) = Command::new("echo").spawn() else { return };
1580+
1581+
// Make sure the process is (un)dead
1582+
match child.kill() {
1583+
// InvalidInput means the child already exited
1584+
Err(e) if e.kind() != ErrorKind::InvalidInput => return,
1585+
_ => {}
1586+
}
1587+
1588+
// open() on this path will succeed, but readdir() will fail
1589+
let id = child.id();
1590+
let path = format!("/proc/{id}/net");
1591+
1592+
// Skip the test if we can't open the directory in the first place
1593+
let Ok(dir) = fs::read_dir(path) else { return };
1594+
1595+
// Check for duplicate errors
1596+
assert!(dir.filter(|e| e.is_err()).take(2).count() < 2);
1597+
}

library/std/src/sys/unix/fs.rs

+19-38
Original file line numberDiff line numberDiff line change
@@ -243,17 +243,15 @@ struct InnerReadDir {
243243

244244
pub struct ReadDir {
245245
inner: Arc<InnerReadDir>,
246-
#[cfg(not(any(
247-
target_os = "android",
248-
target_os = "linux",
249-
target_os = "solaris",
250-
target_os = "illumos",
251-
target_os = "fuchsia",
252-
target_os = "redox",
253-
)))]
254246
end_of_stream: bool,
255247
}
256248

249+
impl ReadDir {
250+
fn new(inner: InnerReadDir) -> Self {
251+
Self { inner: Arc::new(inner), end_of_stream: false }
252+
}
253+
}
254+
257255
struct Dir(*mut libc::DIR);
258256

259257
unsafe impl Send for Dir {}
@@ -594,6 +592,10 @@ impl Iterator for ReadDir {
594592
target_os = "illumos"
595593
))]
596594
fn next(&mut self) -> Option<io::Result<DirEntry>> {
595+
if self.end_of_stream {
596+
return None;
597+
}
598+
597599
unsafe {
598600
loop {
599601
// As of POSIX.1-2017, readdir() is not required to be thread safe; only
@@ -604,8 +606,12 @@ impl Iterator for ReadDir {
604606
super::os::set_errno(0);
605607
let entry_ptr = readdir64(self.inner.dirp.0);
606608
if entry_ptr.is_null() {
607-
// null can mean either the end is reached or an error occurred.
608-
// So we had to clear errno beforehand to check for an error now.
609+
// We either encountered an error, or reached the end. Either way,
610+
// the next call to next() should return None.
611+
self.end_of_stream = true;
612+
613+
// To distinguish between errors and end-of-directory, we had to clear
614+
// errno beforehand to check for an error now.
609615
return match super::os::errno() {
610616
0 => None,
611617
e => Some(Err(Error::from_raw_os_error(e))),
@@ -1363,18 +1369,7 @@ pub fn readdir(path: &Path) -> io::Result<ReadDir> {
13631369
} else {
13641370
let root = path.to_path_buf();
13651371
let inner = InnerReadDir { dirp: Dir(ptr), root };
1366-
Ok(ReadDir {
1367-
inner: Arc::new(inner),
1368-
#[cfg(not(any(
1369-
target_os = "android",
1370-
target_os = "linux",
1371-
target_os = "solaris",
1372-
target_os = "illumos",
1373-
target_os = "fuchsia",
1374-
target_os = "redox",
1375-
)))]
1376-
end_of_stream: false,
1377-
})
1372+
Ok(ReadDir::new(inner))
13781373
}
13791374
}
13801375

@@ -1755,7 +1750,6 @@ mod remove_dir_impl {
17551750
use crate::os::unix::io::{AsRawFd, FromRawFd, IntoRawFd};
17561751
use crate::os::unix::prelude::{OwnedFd, RawFd};
17571752
use crate::path::{Path, PathBuf};
1758-
use crate::sync::Arc;
17591753
use crate::sys::common::small_c_string::run_path_with_cstr;
17601754
use crate::sys::{cvt, cvt_r};
17611755

@@ -1832,21 +1826,8 @@ mod remove_dir_impl {
18321826
// a valid root is not needed because we do not call any functions involving the full path
18331827
// of the DirEntrys.
18341828
let dummy_root = PathBuf::new();
1835-
Ok((
1836-
ReadDir {
1837-
inner: Arc::new(InnerReadDir { dirp, root: dummy_root }),
1838-
#[cfg(not(any(
1839-
target_os = "android",
1840-
target_os = "linux",
1841-
target_os = "solaris",
1842-
target_os = "illumos",
1843-
target_os = "fuchsia",
1844-
target_os = "redox",
1845-
)))]
1846-
end_of_stream: false,
1847-
},
1848-
new_parent_fd,
1849-
))
1829+
let inner = InnerReadDir { dirp, root: dummy_root };
1830+
Ok((ReadDir::new(inner), new_parent_fd))
18501831
}
18511832

18521833
#[cfg(any(

0 commit comments

Comments
 (0)