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

race condition with recursive copy #72

Open
ctron opened this issue Aug 24, 2023 · 1 comment
Open

race condition with recursive copy #72

ctron opened this issue Aug 24, 2023 · 1 comment

Comments

@ctron
Copy link

ctron commented Aug 24, 2023

I believe there is a race condition when recursively copying:

fs_extra/src/dir.rs

Lines 604 to 609 in 1754296

if !dir.exists() {
if options.copy_inside {
create_all(dir, false)?;
} else {
create(dir, false)?;
}

create_all will fail when the directory already exists. Having two threads copying files to the same directory, this may result in an a File exists error.

The my understanding tokio compensates for this by ignoring this case as an error: https://docs.rs/tokio/latest/tokio/fs/fn.create_dir_all.html

Notable exception is made for situations where any of the directories specified in the path could not be created as it was being created concurrently. Such cases are considered to be successful. That is, calling create_dir_all concurrently from multiple threads or processes is guaranteed not to fail due to a race condition with itself.

I think it makes sense to implement the same logic here.

@ctron
Copy link
Author

ctron commented Aug 24, 2023

Digging a bit into this, I guess the issue is more complex.

create_all and create say:

This function will return an error in the following situations, but is not limited to just these cases:

[…]

  • path already exists if erase set false.

However, create_all uses DirBuilder with recursive(true), in which case the docs say:

It is considered an error if the directory already exists unless recursive mode is enabled.

So create_all will succeed with an existing directory, while crate will fail. crate will also fail if erase is true, in case of another race condition.

My proposal would be to simply change the create function to ignore the "directory exists" error.

ctron added a commit to ctron/fs_extra that referenced this issue Aug 24, 2023
Align behavior of dir::create and dir::create_all to not report an
existing directory as error. Fixing race conditions when running
multiple copy operations in parallel.
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

No branches or pull requests

1 participant