Boost C++ Libraries: Ticket #12987: boost::filesystem::exists crashes

anonymous — Tue, 25 Apr 2017 00:24:50 GMT

I encountered the same problem using vc2010 on win10.

daniel.kruegler@… — Wed, 26 Apr 2017 05:46:56 GMT

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@… — Wed, 26 Apr 2017 05:58:54 GMT

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

cc set

mika.fischer@… — Wed, 26 Apr 2017 10:50:14 GMT

cc mika.fischer@… added

Leinad — Wed, 26 Apr 2017 16:31:35 GMT

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@… — Wed, 26 Apr 2017 18:24:18 GMT

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 — Wed, 26 Apr 2017 18:36:08 GMT

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 — Thu, 27 Apr 2017 10:59:56 GMT

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@… — Thu, 27 Apr 2017 11:28:50 GMT

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@… — Sat, 29 Apr 2017 14:50:01 GMT

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 — Wed, 03 May 2017 14:15:36 GMT

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@… — Fri, 18 Aug 2017 17:01:36 GMT

Is upcoming boost 1.65 still affected by this bug ?

cc changed

Fri, 25 Aug 2017 10:03:32 GMT

cc Marcel.Raad@… added

anonymous — Mon, 25 Sep 2017 13:38:48 GMT

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

Tue, 03 Oct 2017 10:11:32 GMT

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

Crl — Wed, 11 Oct 2017 10:20:45 GMT

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@… — Sun, 14 Jan 2018 06:07:39 GMT

This bug is fixed in boost 1.66?

anonymous — Mon, 15 Jan 2018 10:25:13 GMT

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