search

webdesus / fs_extra

Expanding opportunities standard library std::fs and std::io
MIT License
301 stars 47 forks source link

race condition with recursive copy #72

Open ctron opened 1 year ago

ctron commented 1 year ago

I believe there is a race condition when recursively copying:

https://github.com/webdesus/fs_extra/blob/1754296075e7cc4a25feaa876a3f4b9daccc0b98/src/dir.rs#L604-L609

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 commented 1 year ago

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.