search

gulrak / filesystem

An implementation of C++17 std::filesystem for C++11 /C++14/C++17/C++20 on Windows, macOS, Linux and FreeBSD.
MIT License
1.32k stars 168 forks source link

Const method isn't thread safe on Windows #90

Closed phprus closed 3 years ago

phprus commented 3 years ago

Race condition in all function, who call GHC_INLINE const path::string_type& path::native() const, because this method write to mutable variable _native_cache.

Proposal: Switch to native wchar_t storage on Windows. I think this it shouldn't hurt the "UTF-8 Everywhere" philosophy if all other std::string in API are UTF-8 encoded.

gulrak commented 3 years ago

Actually it has not really to do with the question of using wchar_t or not internally, but with the fact that the library uses the generic form internally, so if you don't set GHC_WIN_WSTRING_STRING_TYPE or use fs_std.hpp or fs_std__fwd.hpp headers, and have the std::string/char interface everywhere, the issue is still there on Windows.

One way would be using a mutex but I really would not like to introduce that into filesystem. I need to think about this.

phprus commented 3 years ago

Change the format of the internal path to native?

This is also a performance issue. The WinAPI *W function expects the wchar_t native format path. All calls WinAPI functions in this library use the encoding and format transformation, for example:

GetFileAttributesExW(p.wstring().c_str(), GetFileExInfoStandard, &attr)
GetFileAttributesExW(detail::fromUtf8<std::wstring>(p.u8string()).c_str(), GetFileExInfoStandard, &attr)
gulrak commented 3 years ago

Yeah, that's of course my favorite solution too, and I started working on that yesterday, I just don't know how fast I can provide that, with the free time I have currently available. So I was thinking about a temporary workaround to mitigate the risk until that work is done.

phprus commented 3 years ago

Great news! Thank you!

Possible workaround: Add macro to select fast and non threadsafe implementation (with mutable _native_cache) or slow and thread-safe (without cache).

gulrak commented 3 years ago

Okay, everything is back together, still want to make some additional tests and look into performance, but I chances are, there might be a release v1.5 tomorrow.

gulrak commented 3 years ago

This is now part of release v1.5.0