Modify

Ticket #6320 (closed Bugs: fixed)

Opened 2 years ago

Last modified 6 months ago

creation of path from std::string on Windows (VC10) crashes (access error)

Reported by: aris.basic@… Owned by: bemandawes
Milestone: To Be Determined Component: filesystem
Version: Boost 1.51.0 Severity: Problem
Keywords: Cc:

Description

following app crashes on VC10 (Windows 7) (multibyte and Unicode builds) at point of creation of path (im working around the issue by doing dummy string to wstring conversion) [

wstring r; std::copy(s.begin(),s.end(),r.begin());

]

#include <boost/thread.hpp>
#include <boost/filesystem.hpp>


int main(void)
{
        std::string sPath("c:\\Development");
        
        boost::thread_group tg;

        for(int i = 0; i < 2; i++)
        {
                tg.create_thread([&sPath](){
                        boost::this_thread::sleep(boost::posix_time::milliseconds(10));
                        boost::filesystem::path p(sPath);

                        boost::filesystem::directory_iterator di(p), end;
                        while(di != end)
                                std::cout << (*(di++)).path().string() << std::endl;
                });
        }

        tg.join_all();

        int a;
        std::cin >> a;

}

VC10 CallStack?:

>	msvcp100d.dll!std::codecvt<wchar_t,char,int>::in(int & _State, const char * _First1, const char * _Last1, const char * & _Mid1, wchar_t * _First2, wchar_t * _Last2, wchar_t * & _Mid2)  Line 1521 + 0x1f bytes	C++
 	filesystem_crash.exe!`anonymous namespace'::convert_aux(const char * from, const char * from_end, wchar_t * to, wchar_t * to_end, std::basic_string<wchar_t,std::char_traits<wchar_t>,std::allocator<wchar_t> > & target, const std::codecvt<wchar_t,char,int> & cvt)  Line 84 + 0x25 bytes	C++
 	filesystem_crash.exe!boost::filesystem3::path_traits::convert(const char * from, const char * from_end, std::basic_string<wchar_t,std::char_traits<wchar_t>,std::allocator<wchar_t> > & to, const std::codecvt<wchar_t,char,int> & cvt)  Line 165 + 0x20 bytes	C++
 	filesystem_crash.exe!boost::filesystem3::path_traits::dispatch<std::basic_string<wchar_t,std::char_traits<wchar_t>,std::allocator<wchar_t> > >(const std::basic_string<char,std::char_traits<char>,std::allocator<char> > & c, std::basic_string<wchar_t,std::char_traits<wchar_t>,std::allocator<wchar_t> > & to, const std::codecvt<wchar_t,char,int> & cvt)  Line 174 + 0x7e bytes	C++
 	filesystem_crash.exe!boost::filesystem3::path::path<std::basic_string<char,std::char_traits<char>,std::allocator<char> > >(const std::basic_string<char,std::char_traits<char>,std::allocator<char> > & source, void * __formal)  Line 135 + 0x13 bytes	C++
 	filesystem_crash.exe!`anonymous namespace'::<lambda0>::operator()()  Line 15 + 0x10 bytes	C++
 	filesystem_crash.exe!boost::detail::thread_data<`anonymous namespace'::<lambda0> >::run()  Line 62	C++
 	filesystem_crash.exe!boost::`anonymous namespace'::thread_start_function(void * param)  Line 177	C++
 	msvcr100d.dll!_callthreadstartex()  Line 314 + 0xf bytes	C
 	msvcr100d.dll!_threadstartex(void * ptd)  Line 297	C

Attachments

Change History

comment:1 Changed 2 years ago by wolfgang.fertsak@…

I think the problem is in file filesystem/v3/source/path.cpp in method

const path::codecvt_type *& path::wchar_t_codecvt_facet() {

static const std::codecvt<wchar_t, char, std::mbstate_t> *

facet(

&std::use_facet<std::codecvt<wchar_t, char, std::mbstate_t> >

(path_locale()));

return facet;

}

This method is entered by multiple threads concurrently and the static pointer "facet" gets initialized by the first thread (which takes some time) but the other threads don't wait and just continue while facet is a NULL pointer.

In boost 1.44 this crash didn't occur. It seems that the initialization of global variable const fs::path dot_path(L"."); in filesystem/v3/source/path.cpp resulted in a call of path::wchar_t_codecvt_facet(), so the static "facet" pointer got initialized during program startup while no other threads were running.

In boost 1.48 the initialization of the same globale variable doesn't result in a call of path::wchar_t_codecvt_facet() so race conditions might occur during initialization of the static "facet" pointer (in C++ 03).

comment:2 Changed 2 years ago by bemandawes

  • Status changed from new to closed
  • Resolution set to fixed

(In [76303]) Fix #4889, #6320, Locale codecvt_facet not thread safe on Windows. Move Windows, Mac OS X, locale and codecvt facet back to namespace scope. POSIX except OS X uses local static initialization (IE lazy) to ensure exceptions are catchable if environmental variables are misconfigured and to avoid use of locale("") if not actually used.

comment:3 Changed 18 months ago by anonymous

  • Status changed from closed to reopened
  • Version changed from Boost 1.48.0 to Boost 1.51.0
  • Resolution fixed deleted
  • Severity changed from Showstopper to Problem

This issue still appears to be present, using:

MSVC 2012 (Win7, 32-bit). Statically linked to boost::filesystem.

Just modified the example code to use "C:/" as the directory search path (to ensure existence).

Code asserts in .../libs/filesystem/src/path.cpp, line 888:

#if defined(BOOST_WINDOWS_API) && defined(BOOST_FILESYSTEM_STATIC_LINK)

  const path::codecvt_type& path::codecvt()
  {
    BOOST_ASSERT_MSG(codecvt_facet_ptr(), "codecvt_facet_ptr() facet hasn't been properly initialized");
    return *codecvt_facet_ptr();
  }

comment:4 Changed 16 months ago by jacob.schloss@…

I agree the issue is still present, I am getting crashes due to invalid access inside the path conversion routines. As original poster says issue is that the initialization of static members is not threadsafe in all pre c++11 compilers. This seems to be compiler dependent, gcc seems to default to adding serialization code, but provides -fno-threadsafe-statics to disable,  MSVC seems to not be. I am currently using MSVC 2010.

Although this comment states that the local is thread local

//  The path locale, which is global to the thread, can be changed by the
//  imbue() function. It is initialized to an implementation defined locale.

It looks like the same path object is shared between threads

  std::locale& path_locale()
  {
    static std::locale loc(default_locale());
    return loc;
  }

Adding explicit one time initialization code to the shared local object with boost::call_once works for me.

  void make_loc(std::locale& loc)
  {
	  loc = default_locale();
  }

  std::locale& path_locale()
  {
    static std::locale loc;

    static boost::once_flag once;
    boost::call_once(once, boost::bind(&make_loc, boost::ref(loc)));
    
    return loc;
  }
  void make_wchar_t_codecvt_facet(const path::codecvt_type *& cvt)
  {
	  cvt = &std::use_facet<std::codecvt<wchar_t, char, std::mbstate_t> >
		  (path_locale());
  }

  const path::codecvt_type *& path::wchar_t_codecvt_facet()
  {
   static const std::codecvt<wchar_t, char, std::mbstate_t> *
     facet;

   static boost::once_flag once;
   boost::call_once(once, boost::bind(&make_wchar_t_codecvt_facet, boost::ref(facet)));

   return facet;
  }

Another workaround that seemed to work is to do a path string operation on one thread in advance of multithread operations.

comment:5 Changed 16 months ago by jacob.schloss@…

The call once struct should probably also not be static, I should pull those out.

comment:6 Changed 16 months ago by jacob.schloss@…

Turning on /we4640 to make these warnings errors produces a large number of hits, there are other places where const static "empty path" objects are local to functions.

comment:7 Changed 14 months ago by bemandawes

  • Status changed from reopened to closed
  • Resolution set to fixed

Boost.Filesystem's implementation of path locale and codecvt handling has been rewritten with much simpler, more portable, and hopefully more robust code. The interface has not changed. The rewrite has been applied to svn trunk as of revision 83062. An added reference documentation section (boost-root/libs/filesystem/doc/reference.html#path-Usage) describes class path usage concerns, such as thread data races.

These changes should resolve this issue. If you believe it has not been resolved satisfactorily, please open a new issue rather than reopening this issue. But before you do that, please read boost-root/libs/filesystem/doc/reference.html#path-Usage to be sure that the problem you are seeing is not a manifestation of one of the those concerns. If you do open a new issue, please be specific and supply a test case and lots of details. Just saying something "doesn't work" or "crashes" is not enough!

Thanks,

--Beman

comment:8 Changed 11 months ago by alexey.meteorite@…

I used revision 83062 and it still crashed because of race condition in codecvt(). Call to codecvt() resulted from my call to fs::exists. I used VC9 and static linking.
I uncommented locale_mutex and its locks in codecvt() and imbue(const std::locale& loc), now everything is fine.

So, please, include mutex into next boost release. Or at least provide some build variant with it, that can be turned on.

comment:9 Changed 8 months ago by Vadim Panin

Have not found "boost-root/libs/filesystem/doc/reference.html#path-Usage" section you are referring to. The following workaround works for me:

#ifdef _MSC_VER
		// fix for https://svn.boost.org/trac/boost/ticket/6320
		boost::filesystem::path::imbue( std::locale( "" ) );
#endif

if placed in main().

comment:10 Changed 6 months ago by anonymous

If you're using Boost as a static lib in a DLL you have to apply the fix in the DLLMain example:

#ifdef WIN32
#ifndef WIN32_LEAN_AND_MEAN
#define WIN32_LEAN_AND_MEAN
#endif
#include <Windows.h>
#include <boost/filesystem/path.hpp>

static void init_boost () {
  boost::filesystem::path p("dummy");
};

BOOL APIENTRY DllMain( HMODULE /* hModule */,
                       DWORD  ul_reason_for_call,
                       LPVOID /* lpReserved */)
{
  switch (ul_reason_for_call)
  {
    case DLL_PROCESS_ATTACH:
      init_boost();
      break;
    case DLL_THREAD_ATTACH:
    case DLL_THREAD_DETACH:
    case DLL_PROCESS_DETACH:
    break;
  }
  return TRUE;
}
#endif
View

Add a comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
The resolution will be deleted. Next status will be 'reopened'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.