microsoft / microsoft/STL

`<filesystem>`: `rename` sometimes deletes the target file.

Open
#5,501 8 comments 1 reaction 0 assignees View on GitHub

Nobody has claimed this yet.

bug filesystem
Dominant language
C++
Stars
11.1k
Forks
1.7k
Avg merge
4d 15h
Merged PRs (30d)
22

Description

Describe the bug

When executing std::filesystem::rename() in parallel with readers, readers will sometimes fail because the file is missing.

This was found while testing code for atomic file changes, where file foo is modified by generating a new foo.<guid> file in the same directory with the new contents, followed by renaming foo.<guid> to foo. What is observed is that sometimes readers fail with error 2 (ERROR_FILE_NOT_FOUND).

I've tested the repro below in a few different systems, some on Windows Server 2019, some on Windows Server 2022, always on NTFS filesystems and always with 32 cores (in reality 16 hyperthreaded cores) -- I expect that the hardcoded 16 threads from the code below may not be ideal everywhere, but, not having access to other systems, I was hesitant to try and make it dynamic.

It usually takes from a few seconds to a few minutes to see the it fail. Procmon seems to slow everything down, I haven't been able to reproduce the issue with it running.

Command-line test case

C:\Temp>type repro.cpp
#include <Windows.h>

#include <atomic>
#include <filesystem>
#include <iostream>
#include <list>
#include <string>
#include <thread>
#include <vector>

const std::filesystem::path fname{L"foo"};
const std::string contents(1000000, 'x');


void read() {
    static thread_local std::vector<char> buf(contents.size() + 1, '\0');
    const auto handle = CreateFileW(
        fname.c_str(),
        GENERIC_READ,
        FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE,
        NULL,
        OPEN_EXISTING,
        FILE_ATTRIBUTE_NORMAL,
        NULL
    );
    if (handle == INVALID_HANDLE_VALUE) {
        std::wcerr << L"Error opening for reading: " << GetLastError() << std::endl;
        exit(1);
    }
    DWORD bytesRead = 0;
    auto success = ReadFile(handle, buf.data(), static_cast<DWORD>(contents.size()), &bytesRead, nullptr);
    CloseHandle(handle);
    if (success == FALSE || bytesRead != contents.size()) {// || contents != buf.data()) {
        std::wcerr << L"Error reading, success=" << success <<
            L", bytesRead=" << bytesRead <<
            L", contents.size()=" << contents.size() <<
            L", err=" << GetLastError() << std::endl;
        exit(1);
    }
}

bool dumpContentsTo(const std::filesystem::path& dest) {
    const auto handle = CreateFileW(
        dest.c_str(),
        GENERIC_WRITE,
        0,
        NULL,
        CREATE_NEW,
        FILE_ATTRIBUTE_NORMAL,
        NULL);
    if (handle == INVALID_HANDLE_VALUE)
        return false;

    DWORD bytesWritten = 0;
    auto success = WriteFile(handle, contents.c_str(), static_cast<DWORD>(contents.size()), &bytesWritten, nullptr);
    CloseHandle(handle);
    return success && bytesWritten == contents.size();
}

unsigned numericThreadId() {
    static std::atomic<unsigned> tid{ 0 };
    static thread_local unsigned id = tid.fetch_add(1);
    return id;
}

void write(const std::filesystem::path& tmpFname) {
    if (!dumpContentsTo(tmpFname))
        return;

    for (unsigned i = 0; i<30; ++i) {
        std::error_code ec;
        std::filesystem::rename(tmpFname, fname, ec);
        if (!ec)
            return;
    }
    std::filesystem::remove(tmpFname);
}

void work() {
    const std::filesystem::path myFname(fname.native() + L'.' + std::to_wstring(numericThreadId()));
    std::filesystem::remove(myFname);

    std::atomic<unsigned> coin;
    while (true) {
        if (coin.fetch_add(1, std::memory_order_relaxed) & 1)
            read();
        else
            write(myFname);
    }
}

int main()
{
    std::filesystem::remove(fname);
    if (!dumpContentsTo(fname)) {
        std::wcerr << L"Critical failure creating " << fname << std::endl;
        return 1;
    }

    std::list<std::thread> ts;
    for (unsigned i = 0; i < 16; ++i)
        ts.emplace_back(&work);

    for (auto& t : ts)
        t.join();

    return 0;
}

C:\Temp>cl /EHsc /W4 /WX /std:c++latest .\repro.cpp
Microsoft (R) C/C++ Optimizing Compiler Version 19.43.34810 for x86
Copyright (C) Microsoft Corporation.  All rights reserved.

/std:c++latest is provided as a preview of language features from the latest C++
working draft, and we're eager to hear about bugs and suggestions for improvements.
However, note that these features are provided as-is without support, and subject
to changes or removal as the working draft evolves. See
https://go.microsoft.com/fwlink/?linkid=2045807 for details.

repro.cpp
Microsoft (R) Incremental Linker Version 14.43.34810.0
Copyright (C) Microsoft Corporation.  All rights reserved.

/out:repro.exe
repro.obj

C:\Temp>.\repro.exe
Error opening for reading: 2

Expected behavior

std::filesystem::rename() must behave like the POSIX rename() function:

If the link named by the new argument exists, it shall be removed and old renamed to new. In this case, a link named new shall remain visible to other threads throughout the renaming operation and refer either to the file referred to by new or old before the operation began.

STL version

  • Option 1: Visual Studio version
Microsoft Visual Studio Professional 2022
Version 17.13.6

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by compiling and running the Windows repro in repro.cpp on NTFS, then trace the implementation of std::filesystem::rename used by the STL. Compare its replacement behavior with the POSIX guarantee described in the issue. Done means concurrent readers no longer receive ERROR_FILE_NOT_FOUND while the target is replaced, with coverage for the regression.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
operating-systems
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.