boost::filesystem::exists crashes

[Leinad]

bool dummy = boost::filesystem::exists("C:\\alma.txt");	//1

class Alma
{
public:
	~Alma() { std::cout << boost::filesystem::path("c:\\alma.txt").string() << std::endl; }
};

Alma a;	//2

int main(int argc, char* argv[])
{
	Alma();	//3
	return 0;
}

The reason for the crash is that "dummy" is initialised before "equal_string_ordinal_ic" in the unnamed namespace in operations.cpp. The fix (or a possible fix) is easy. I made operations.cpp a bit uglier by moving static stuff to "perms make_permissions(const path&, DWORD)":

perms make_permissions(const path& p, DWORD attr)
  {
    //this was in the unnamed namespace:
    static PtrRtlEqualUnicodeString rtl_equal_unicode_string_api = PtrRtlEqualUnicodeString(
      ::GetProcAddress(
        ::GetModuleHandleW(L"ntdll.dll"), "RtlEqualUnicodeString"));

    //this was in the unnamed namespace:
    static bool(*equal_string_ordinal_ic)(const wchar_t*, const wchar_t*, PtrRtlEqualUnicodeString) =
      rtl_equal_unicode_string_api ? equal_string_ordinal_ic_1 : equal_string_ordinal_ic_2;

    perms prms = fs::owner_read | fs::group_read | fs::others_read;
    if  ((attr & FILE_ATTRIBUTE_READONLY) == 0)
      prms |= fs::owner_write | fs::group_write | fs::others_write;
    path ext = p.extension();
    if (equal_string_ordinal_ic(ext.c_str(), L".exe", rtl_equal_unicode_string_api)
      || equal_string_ordinal_ic(ext.c_str(), L".com", rtl_equal_unicode_string_api)
      || equal_string_ordinal_ic(ext.c_str(), L".bat", rtl_equal_unicode_string_api)
      || equal_string_ordinal_ic(ext.c_str(), L".cmd", rtl_equal_unicode_string_api))
      prms |= fs::owner_exe | fs::group_exe | fs::others_exe;
    return prms;
  }

I also changed the signatures of two locally used functions:

bool equal_string_ordinal_ic_1(const wchar_t* s1, const wchar_t* s2, PtrRtlEqualUnicodeString rtl_equal_unicode_string_api)

bool equal_string_ordinal_ic_2(const wchar_t* s1, const wchar_t* s2, PtrRtlEqualUnicodeString)

And now that the static stuff is moved from the unnamed namespace to the make_permissions method there is no crash anymore.

At least until I comment out (1). Please note that (3) is needed for the second type of crash. This second type of crash is not entirely new. There is a bug filed 5 years ago. I think someone who is involved in developing boost::filesystem with this info (that the second type of crash does not occur if we call (1) (supposed make_permissions has been fixed)) could sort out that old bug as well. ​https://svn.boost.org/trac/boost/ticket/6638

This static initialisation/destruction looks a nasty business.

compiler: vc14, static lib, statically linked runtime

[anonymous]

I encountered the same problem using vc2010 on win10.

[daniel.kruegler@…]

I will work on a solution. We also need to ensure that static local function initialization is thread-safe, which is not meet before VS 2017, therefore your suggested workaround has some limitations.

[daniel.kruegler@…]

Correction: I meant VS 2013 instead of VS 2017 in regard to thread-safe function statics.

[Leinad]

Yes that's correct. I think it's a c++11 feature, but we use this already in path.cpp:

 std::locale& path_locale()
  // std::locale("") construction, needed on non-Apple POSIX systems, can throw
  // (if environmental variables LC_MESSAGES or LANG are wrong, for example), so
  // path_locale() provides lazy initialization via a local static to ensure that any 
  // exceptions occur after main() starts and so can be caught. Furthermore,
  // path_locale() is only called if path::codecvt() or path::imbue() are themselves
  // actually called, ensuring that an exception will only be thrown if std::locale("")
  // is really needed.
  {
    // [locale] paragraph 6: Once a facet reference is obtained from a locale object by
    // calling use_facet<>, that reference remains usable, and the results from member
    // functions of it may be cached and re-used, as long as some locale object refers
    // to that facet.
    static std::locale loc(default_locale());
#ifdef BOOST_FILESYSTEM_DEBUG
    std::cout << "***** path_locale() called" << std::endl;
#endif
    return loc;
  }

[daniel.kruegler@…]

I currently tend to favour to accept this pull request:

​https://github.com/boostorg/filesystem/pull/43

It has the advantage to localize the algorithm decision logic and doesn't need to change the calling signature of equal_string_ordinal_ic in such an ugly way. I would like to test it tomorrow on different systems.

[anonymous]

Replying to Leinad:

Yes that's correct. I think it's a c++11 feature, but we use this already in path.cpp:

 std::locale& path_locale()
    [...]
    return loc;
  }

Exactly this function causes problems as described in issue #6320.

[Leinad]

Replying to daniel.kruegler@…:

I currently tend to favour to accept this pull request:

​https://github.com/boostorg/filesystem/pull/43

It has the advantage to localize the algorithm decision logic and doesn't need to change the calling signature of equal_string_ordinal_ic in such an ugly way. I would like to test it tomorrow on different systems.

I agree. That solution (pull request 43) eliminates the ugliness of the signature of equal_string_ordinal_ic, but is it any different in terms of local static initialisation thread safeness? Would it have the same limitations?

[daniel.kruegler@…]

Replying to Leinad:

I agree. That solution (pull request 43) eliminates the ugliness of the signature of equal_string_ordinal_ic, but is it any different in terms of local static initialisation thread safeness? Would it have the same limitations?

IMO it has similar limitations and I'm working on a variant of that proposal that fixes the thread-safeness problem.

[daniel.kruegler@…]

Replying to daniel.kruegler@…:

Replying to Leinad:

I agree. That solution (pull request 43) eliminates the ugliness of the signature of equal_string_ordinal_ic, but is it any different in terms of local static initialisation thread safeness? Would it have the same limitations?

IMO it has similar limitations and I'm working on a variant of that proposal that fixes the thread-safeness problem.

Daniela has now provided a revised resolution (very similar to her original one), which realizes that initialization happens in the (dynamic) program initialization phase (as the original approach did) and doesn't depend on local statics:

​https://github.com/boostorg/filesystem/pull/43/commits/24240ed577229eb9568fd8050b9ca4233be92116

It has been successfully tested against Visual Studio versions 2008, 2010, 2012, 2013, 2015, and 2017 and against MinGW with gcc 8 HEAD. I recommend to accept that PR.

[anonymous]

Is it thread safe?

#include <boost\filesystem.hpp>

const int thread_num = 10;
boost::barrier barrier(thread_num + 1);

bool c = []
{
	std::vector<std::thread> threads;
	for (int i = 0; i < thread_num; ++i)
	{
		threads.emplace_back([]
		{
			barrier.wait();
			boost::filesystem::exists("c:");
		});
	}
	barrier.wait();
	for (auto& thread: threads)
		thread.join();
	return true;
}();

int main(int argc, char* argv[])
{
	return 0;
}

c could be (probably is) initialised before compare_function_initializer in the unnamed namespace in operations.cpp. All those threads will call compare() and there might be race condition at reading/setting compare_function.

I guess on many platforms reading/assigning a pointer is atomic in practice, and GetProcAddress and GetModuleHandle are thread-safe, too, so this is not an issue here.

[o.tristan@…]

Is upcoming boost 1.65 still affected by this bug ?

[anonymous]

I'm using boost 1.65 with VS2015 and I still have this problem.

[Ilia <ki.stfu@…>]

I'm using boost 1.65.1 with VS2017 and I still have this problem.

[Crl]

We switched from 1.63 to 1.65.1 recently and we also encounter this issue. Is there a workaround? When can we expect a patch for this issue?

[pericles@…]

This bug is fixed in boost 1.66?

[anonymous]

Nop. But will probably be in 1.67 See ​https://github.com/boostorg/filesystem/pull/43