Const method isn't thread safe on Windows
phprus opened this issue · 6 comments
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.
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.
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)
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.
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).
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.