KEMBAR78
Fix `filesystem::directory_entry::refresh` on Win11 24H2 by StephanTLavavej · Pull Request #5077 · microsoft/STL · GitHub
Skip to content

Conversation

@StephanTLavavej
Copy link
Member

After upgrading to Win11 24H2, I observed a very sporadic failure in P0218R1_filesystem. (#4844 was also a behavioral change in 24H2, but that one was consistent.) Eventually I was able to reproduce this with focused testing:

Click to expand reduced test case:
D:\GitHub\STL\out\x64>type meow.cpp
#include <cassert>
#include <exception>
#include <filesystem>
#include <iostream>
#include <random>
#include <string>
#include <system_error>
using namespace std;
using namespace std::filesystem;

bool good(const error_code& ec) {
    bool overall = true;
    if (ec.value() != 0) {
        wcout << L"Unexpected error " << ec.value() << L" " << ec.message().c_str() << L"\n";
        overall = false;
    }

    if (ec.category() != system_category()) {
        wcout << L"Unexpected category " << ec.category().name() << L"\n";
        overall = false;
    }

    return overall;
}

int main() {
    for (int repeat = 0; repeat < 10'000; ++repeat) {
        try {
            random_device rd;

            wstring hexits;

            if ((rd() & 0xFF) == 0) {
                hexits = L"woof";
            } else {
                for (int i = 0; i < 32; ++i) {
                    hexits.push_back(L"0123456789abcdef"[rd() & 0xF]);
                }
            }

            error_code ec;

            const path nonexistent{L"//this_path_does_not_exist_on_the_network_" + hexits + L"/docs"};

            directory_entry nonexistentEntry(nonexistent);
            assert(nonexistentEntry.path() == nonexistent);
            // Test VSO-892890 "std::filesystem::directory_entry constructor initializes wrong state"
            assert(!nonexistentEntry.exists());

            directory_entry nonexistentEntryEc(nonexistent, ec);
            assert(nonexistentEntryEc.path() == nonexistent);
            // Test VSO-892890 "std::filesystem::directory_entry constructor initializes wrong state"
            assert(!nonexistentEntryEc.exists());
            assert(good(ec));

            // Also test GH-232 "<filesystem>: directory_entry(const path& p, error_code& ec) does not return error
            // code"
            nonexistentEntry.refresh();

            nonexistentEntryEc.refresh(ec);
            assert(good(ec));
        } catch (const filesystem_error& fe) {
            cout << "Caught filesystem_error." << endl;
            cout << "    what: " << fe.what() << endl;
            cout << "   value: " << fe.code().value() << endl;
            cout << "category: " << fe.code().category().name() << endl;
        } catch (const exception& e) {
            cout << "Caught exception." << endl;
            cout << "what: " << e.what() << endl;
        } catch (...) {
            cout << "Caught unknown exception." << endl;
        }
    }
}
D:\GitHub\STL\out\x64>cl /EHsc /nologo /W4 /std:c++latest /MTd /Od meow.cpp && meow
meow.cpp

D:\GitHub\STL\out\x64>meow
Caught filesystem_error.
    what: directory_entry::refresh: The specified network name is no longer available.: "//this_path_does_not_exist_on_the_network_woof/docs"
   value: 64
category: system

According to System Error Codes (0-499), this is:

ERROR_NETNAME_DELETED
64 (0x40)
The specified network name is no longer available.

Like in #4844, we need to teach __std_is_file_not_found to recognize this error code.

(I don't know, and don't really care, what the root cause is. In my reduced test case, I randomly switch between random hexits and "woof" in the nonexistent server name, and only the "woof" case seems to fail. I suspect that this is somehow related to filesystem caching, where repeatedly querying for a network name's existence can sometimes return this ERROR_NETNAME_DELETED error code.)

Finally, #4844 enhanced P0218R1_filesystem, which was very helpful here, but not quite enough:

Additionally, this uncaught exception from a non-error_code overload manifested itself as an abort() with no useful logs. To aid in future investigations, I'm including a simple change to the filesystem test. Now, we wrap the whole test in try ... catch and print the contents of any exception before failing.

In this case, I really needed to know the numeric error code, not just the message, so I'm further enhancing the test coverage to print the filesystem_error's error code value and category (expected to be the system category). I verified that with this test change, but without the product code fix, the sporadic failure prints:

Caught filesystem_error.
    what: directory_entry::refresh: The specified network name is no longer available.: "//this_path_does_not_exist_on_the_network_e9da301701f70ead24c65bd30f600d15/docs"
   value: 64
category: system

@StephanTLavavej StephanTLavavej added bug Something isn't working filesystem C++17 filesystem labels Nov 11, 2024
@StephanTLavavej StephanTLavavej requested a review from a team as a code owner November 11, 2024 18:20
@CaseyCarter CaseyCarter removed their assignment Nov 11, 2024
@StephanTLavavej StephanTLavavej self-assigned this Nov 14, 2024
@StephanTLavavej
Copy link
Member Author

I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed.

@StephanTLavavej StephanTLavavej merged commit dec6569 into microsoft:main Nov 14, 2024
39 checks passed
@StephanTLavavej StephanTLavavej deleted the fix-directory_entry-refresh branch November 14, 2024 20:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working filesystem C++17 filesystem

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

2 participants