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

Fix filesystem::directory_entry::refresh on Win11 24H2 #5077

Merged

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