Skip to content
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

std::fs::create_dir_all will stack overflow with big paths #124309

Open
xTachyon opened this issue Apr 23, 2024 · 5 comments
Open

std::fs::create_dir_all will stack overflow with big paths #124309

xTachyon opened this issue Apr 23, 2024 · 5 comments
Assignees
Labels
A-io Area: std::io, std::fs, std::net and std::path C-bug Category: This is a bug. T-libs Relevant to the library team, which will review and decide on the PR/issue.

Comments

@xTachyon
Copy link

xTachyon commented Apr 23, 2024

I tried this code:

std::fs::create_dir_all(big_path);

I expected to see this happen: doesn't stack overflow

Instead, this happened:
std::fs::create_dir_all with a big path with many uncreated directories will stack overflow on Windows. The max path is 32767, which can technically result in ~16383 recursive calls, which will stack overflow the 1mb default stack on Windows.

Meta

rustc --version --verbose:

rustc 1.77.1 (7cf61ebde 2024-03-27)
binary: rustc
commit-hash: 7cf61ebde7b22796c69757901dd346d0fe70bd97
commit-date: 2024-03-27
host: x86_64-pc-windows-msvc
release: 1.77.1
LLVM version: 17.0.6
@xTachyon xTachyon added the C-bug Category: This is a bug. label Apr 23, 2024
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Apr 23, 2024
@ChrisDenton
Copy link
Contributor

I can see two ways of fixing this. Change this so it's not recursive;

rust/library/std/src/fs.rs

Lines 2601 to 2613 in 40dcd79

fn create_dir_all(&self, path: &Path) -> io::Result<()> {
if path == Path::new("") {
return Ok(());
}
match self.inner.mkdir(path) {
Ok(()) => return Ok(()),
Err(ref e) if e.kind() == io::ErrorKind::NotFound => {}
Err(_) if path.is_dir() => return Ok(()),
Err(e) => return Err(e),
}
match path.parent() {
Some(p) => self.create_dir_all(p)?,

Or defer to platform-specific implementations so that they can use the most efficient method for their platform.

@ChrisDenton ChrisDenton added T-libs Relevant to the library team, which will review and decide on the PR/issue. A-io Area: std::io, std::fs, std::net and std::path and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Apr 23, 2024
@xTachyon
Copy link
Author

I did strace mkdir -p .. on a Linux and it doesn't seem to do anything fancier than just calling the mkdir syscall with each component.
Doing the equivalent under Windows (but with procmon) shows repeated calls to CreateFile and CloseFile with each component.

So I'm not sure there's a better way to do this in a platform specific way. Looks like the correct solution would be to just do it iteratively.

As a side note, deleting that huge directory tree (that looks like a/a/a/a/.../a) took 38m18.119s 😰 . Even Windows is handling this terribly.

@ChrisDenton
Copy link
Contributor

ChrisDenton commented Apr 24, 2024

On that side note, removing it didn't seem that slow on my machine. I kept pushing a to a PathBuf in a loop, using std::fs::create_dir each time to make the directory tree as deep as possible until it errored (this was incredibly slow). Deleting it with std::fs::remove_dir_all on Windows 11 took and about 3.8s on a ReFs drive and about 3 minutes on an NTFS drive.

@xTachyon
Copy link
Author

I'm on W10. Maybe W11 got better at doing this? Or the rm command from MSYS2 doesn't play well with how Windows expects things to be done.

@xTachyon
Copy link
Author

@rustbot claim

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-io Area: std::io, std::fs, std::net and std::path C-bug Category: This is a bug. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

3 participants