Boost C++ Libraries: Ticket #11166: boost::filesystem::remove is racy

attachment set

Fri, 03 Apr 2015 20:05:21 GMT

attachment → fsremoverace.cc

demonstrate race in boost::filesystem::remove

Fri, 03 Apr 2015 20:08:57 GMT

It's also interesting to contemplate that the change might not be [directory/file/symlink] -> [nonexistent], but could also be [directory/file/symlink] -> [directory/file/symlink].

attachment set

Mon, 06 Apr 2015 14:50:08 GMT

attachment → boost-11166-fs-remove-race.patch

Proposed fix

Mon, 06 Apr 2015 14:58:27 GMT

The proposed patch fixes the file -> nonexistent and directory -> nonexistent cases. The others are not as important for our use case, but for example (errno == EISDIR) would be an appropriate check after unlink() for the file -> directory case. Lightly tested via my test case on Linux and Windows.

Mon, 06 Apr 2015 21:00:11 GMT

This bug may be easier to hit than I first supposed. We have now received some information that this may be exacerbated by the "SMB2 Client Redirector cache" (Windows, Vista and later), which by default will cache positive existence of a file for 10 seconds, making a nice big window for trouble as long as two machines sharing the same Windows share are involved. A similar design choice on Unix with NFS (attribute caching) probably creates nice big windows there too.

status changed; resolution set

Beman Dawes — Tue, 01 Sep 2015 15:34:56 GMT

status new → closed
resolution → fixed

A change similar to the suggested patch has been applied. It covers the same set of error codes used elsewhere to identify the general "file not found" case.

Please note that this change mitigates but does not totally eliminate the chance of external file system races.

It does not protect against multi-threading data races. "Data races are bad", to quote Howard Hinnant and a bunch of other experts. For rationale, see https://software.intel.com/en-us/blogs/2013/01/06/benign-data-races-what-could-possibly-go-wrong

The test program provide has both a file system race and a data race, so it always fails for me (four core CPU, Windows 10, 64-bit system, VC++ 2015, Boost 1.59) because of the data race regardless of anything done to mitigate the file system race.

Thanks for the report, example program, and patch,

--Beman

anonymous — Tue, 01 Sep 2015 16:24:38 GMT

I appreciate your comments about the safety of threaded code. I provided a threaded program for ease of reproduction, and may have not adequately observed threaded programming safeguards.

In our use case, the real how-to-reproduce involves multiple processes, often on different systems with a networked file system, concurrently creating and deleting files.

Since these activities are human driven, we can't enforce that two users don't both delete the same item from the filesystem at close enough to the same time that the cached existence in the SMB2 Client Redirector Cache doesn't trigger this issue.