Bug #1065
Is fileProtocol() thread-safe?
100%
Description
I use libexiv2 in multi-threaded app (2 parallel threads) and sometimes get a crash on function fileProtocol(). I see in debugger that both threads are in this proc and trying to fill the protDict var. Here is the trace.
First thread:
* thread #15: tid = 0xddef48, 0x00007fff8efe128e libstdc++.6.dylib`std::string::compare(std::string const&) const + 10, name = 'DataCacherThread'
frame #0: 0x00007fff8efe128e libstdc++.6.dylib`std::string::compare(std::string const&) const + 10
frame #1: 0x00000001007ad6ed libexiv2.dylib`bool std::operator<<char, std::char_traits<char>, std::allocator<char> >(__lhs=0x0000000100adb558, __rhs=0x0000000108103498) + 29 at basic_string.h:2228
frame #2: 0x00000001007ad5c1 libexiv2.dylib`std::less<std::string>::operator(this=0x0000000100adb530, __x=0x0000000100adb558, __y=0x0000000108103498)(std::string const&, std::string const&) const + 33 at stl_function.h:227
frame #3: 0x000000010080ff0d libexiv2.dylib`std::_Rb_tree<std::string, std::pair<std::string const, Exiv2::Protocol>, std::_Select1st<std::pair<std::string const, Exiv2::Protocol> >, std::less<std::string>, std::allocator<std::pair<std::string const, Exiv2::Protocol> > >::_M_insert_unique(this=0x0000000100adb530, __position=std::_Rb_tree<std::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::pair<const std::basic_string<char, std::char_traits<char>, std::allocator<char> >, Exiv2::Protocol>, std::_Select1st<std::pair<const std::basic_string<char, std::char_traits<char>, std::allocator<char> >, Exiv2::Protocol> >, std::less<std::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<const std::basic_string<char, std::char_traits<char>, std::allocator<char> >, Exiv2::Protocol> > >::iterator at 0x00000001081033e8, __v=0x0000000108103498) + 493 at stl_tree.h:1019
frame #4: 0x000000010080fc5d libexiv2.dylib`std::map<std::string, Exiv2::Protocol, std::less<std::string>, std::allocator<std::pair<std::string const, Exiv2::Protocol> > >::insert(this=0x0000000100adb530, __position=std::map<std::basic_string<char, std::char_traits<char>, std::allocator<char> >, Exiv2::Protocol, std::less<std::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<const std::basic_string<char, std::char_traits<char>, std::allocator<char> >, Exiv2::Protocol> > >::iterator at 0x0000000108103428, __x=0x0000000108103498) + 45 at stl_map.h:427
frame #5: 0x000000010080f097 libexiv2.dylib`std::map<std::string, Exiv2::Protocol, std::less<std::string>, std::allocator<std::pair<std::string const, Exiv2::Protocol> > >::operator[](this=0x0000000100adb530, __k=0x00000001081035a8) + 199 at stl_map.h:350
* frame #6: 0x000000010080d84d libexiv2.dylib`Exiv2::fileProtocol(path=0x0000000108103868) + 989 at futils.cpp:252
frame #7: 0x000000010081831d libexiv2.dylib`Exiv2::ImageFactory::createIo(path=0x0000000108103868, useCurl=false) + 45 at image.cpp:425
frame #8: 0x0000000100818650 libexiv2.dylib`Exiv2::ImageFactory::open(path=0x0000000108103868, useCurl=false) + 64 at image.cpp:473
frame #6: 0x000000010080d84d libexiv2.dylib`Exiv2::fileProtocol(path=0x0000000108103868) + 989 at futils.cpp:252
249 protDict["ssh://" ] = pSsh;
250 protDict["file://" ] = pFileUri;
251 protDict["data:" ] = pDataUri;
-> 252 protDict["-" ] = pStdin;
253 }
254 for (Exiv2::protDict_i it = protDict.begin(); it != protDict.end(); it++) {
255 if (path.find(it->first) == 0)
Second thread:
* thread #16: tid = 0xddef49, 0x00007fff8efe128e libstdc++.6.dylib`std::string::compare(std::string const&) const + 10, name = 'DataCacherThread', stop reason = EXC_BAD_ACCESS (code=1, address=0xfffffffffffffff2)
frame #0: 0x00007fff8efe128e libstdc++.6.dylib`std::string::compare(std::string const&) const + 10
frame #1: 0x00000001007ad6ed libexiv2.dylib`bool std::operator<<char, std::char_traits<char>, std::allocator<char> >(__lhs=0x0000000100adb558, __rhs=0x000000010ad09498) + 29 at basic_string.h:2228
* frame #2: 0x00000001007ad5c1 libexiv2.dylib`std::less<std::string>::operator(this=0x0000000100adb530, __x=0x0000000100adb558, __y=0x000000010ad09498)(std::string const&, std::string const&) const + 33 at stl_function.h:227
frame #3: 0x000000010080ff0d libexiv2.dylib`std::_Rb_tree<std::string, std::pair<std::string const, Exiv2::Protocol>, std::_Select1st<std::pair<std::string const, Exiv2::Protocol> >, std::less<std::string>, std::allocator<std::pair<std::string const, Exiv2::Protocol> > >::_M_insert_unique(this=0x0000000100adb530, __position=std::_Rb_tree<std::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::pair<const std::basic_string<char, std::char_traits<char>, std::allocator<char> >, Exiv2::Protocol>, std::_Select1st<std::pair<const std::basic_string<char, std::char_traits<char>, std::allocator<char> >, Exiv2::Protocol> >, std::less<std::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<const std::basic_string<char, std::char_traits<char>, std::allocator<char> >, Exiv2::Protocol> > >::iterator at 0x000000010ad093e8, __v=0x000000010ad09498) + 493 at stl_tree.h:1019
frame #4: 0x000000010080fc5d libexiv2.dylib`std::map<std::string, Exiv2::Protocol, std::less<std::string>, std::allocator<std::pair<std::string const, Exiv2::Protocol> > >::insert(this=0x0000000100adb530, __position=std::map<std::basic_string<char, std::char_traits<char>, std::allocator<char> >, Exiv2::Protocol, std::less<std::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<const std::basic_string<char, std::char_traits<char>, std::allocator<char> >, Exiv2::Protocol> > >::iterator at 0x000000010ad09428, __x=0x000000010ad09498) + 45 at stl_map.h:427
frame #5: 0x000000010080f097 libexiv2.dylib`std::map<std::string, Exiv2::Protocol, std::less<std::string>, std::allocator<std::pair<std::string const, Exiv2::Protocol> > >::operator[](this=0x0000000100adb530, __k=0x000000010ad095a8) + 199 at stl_map.h:350
frame #6: 0x000000010080d84d libexiv2.dylib`Exiv2::fileProtocol(path=0x000000010ad09868) + 989 at futils.cpp:252
frame #7: 0x000000010081831d libexiv2.dylib`Exiv2::ImageFactory::createIo(path=0x000000010ad09868, useCurl=false) + 45 at image.cpp:425
frame #8: 0x0000000100818650 libexiv2.dylib`Exiv2::ImageFactory::open(path=0x000000010ad09868, useCurl=false) + 64 at image.cpp:473
frame #6: 0x000000010080d84d libexiv2.dylib`Exiv2::fileProtocol(path=0x000000010ad09868) + 989 at futils.cpp:252
249 protDict["ssh://" ] = pSsh;
250 protDict["file://" ] = pFileUri;
251 protDict["data:" ] = pDataUri;
-> 252 protDict["-" ] = pStdin;
253 }
254 for (Exiv2::protDict_i it = protDict.begin(); it != protDict.end(); it++) {
255 if (path.find(it->first) == 0)
Related issues
Associated revisions
#1065. Thanks to Thomas B for spotting my error in omitting support for https.
#1065. std::map<int,const char*> doesn't build on MacOS-X 10.6 (Snow Leopard). Thanks Max for letting me know about this.
History
Updated by Robin Mills over 6 years ago
- Subject changed from Does fileProtocol() thread-safe? to Is fileProtocol() thread-safe?
- Category set to basicio
- Status changed from New to Assigned
- Assignee set to Robin Mills
- Target version set to 0.25
Max
You are right. It is not thread safe. It builds a little static map when it's run for the first time. If two threads hit it while it's being built - bad things can happen. I'll have to think about how best to have the dictionary built. We have tried to avoid having library initialization code. I'd also like to see if there are other little threading bombs hiding nearby!
My suggested work around for now is to call fileProtocol("") before you start spinning up threads and you'll probably be good.
Updated by Robin Mills over 6 years ago
Fix submitted. r3729. Thank you, Max for reporting this.
I've eliminated the (horrible) static and replaced it with a constant compiled-in array of static data. No need to initialize anything. I've reviewed the other two statics in futils.cpp and they are true static strings used by hex and base64 code.
Max:
I'm very pleased that you have a multi-threaded application. I won't mark this issue as "Resolved" at the moment. If/when you encounter additional thread-safety issues, please update this issue report and I will give it attention.
Updated by Thomas Beutlich over 6 years ago
Any reason why pHttps was removed by r3729 from the map?
Updated by Robin Mills over 6 years ago
Yes, there's a reason. I'm dyslexic. I counted the protocols in the Protocols enum in futils.hpp and decided everything was OK. Thank Goodness you're awake Thomas. Well spotted. Fix submitted r3730.
Updated by Max Pozdeev over 6 years ago
Thank you for quick response. I'll do more tests, I want to use multi-threading in production.
Updated by Max Pozdeev over 6 years ago
Just for info. I can not build latest revision (r3730) on Mac OSX 10.10 64 bit using "-stdlib=libstdc++ -mmacosx-version-min=10.6". The error is missing std::unique_ptr.
tiffimage.cpp:347:8: error: no member named 'unique_ptr' in namespace 'std'
std::unique_ptr<char[]> formatted;
~~~~~^
...
1 warning and 6 errors generated.
$ g++ -v Configured with: --prefix=/Applications/Xcode.app/Contents/Developer/usr --with-gxx-include-dir=/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.9.sdk/usr/include/c++/4.2.1 Apple LLVM version 5.1 (clang-503.0.40) (based on LLVM 3.4svn) Target: x86_64-apple-darwin14.3.0 Thread model: posix
The fix is to use "-stdlib=libc++ -mmacosx-version-min=10.7", but the minimum system version will be higher - 10.7.
Updated by Robin Mills over 6 years ago
Oh, that's odd. I'm also working on MacOS-X. I haven't built on 10.6 (Snow Leopard) for years. I know what's causing that. It's code that I found on StackOverflow. Can I ask you to raise a different bug about building on 10.6 and I'll deal with it tomorrow. I've reproduced your build failure with the command:
env MACOSX_DEPLOYMENT_TARGET=10.6 make rebuild ; exiv2 -vV -g date -g time -t svnPerhaps you could check 10.7, 10.8 and 10.9. As they could be building with GCC or Clang. Best to know the extent of the issue. Here's what's building OK for me:
773 rmills@rmillsmbp:~/gnu/exiv2/trunk $ date ; sw_vers ; g++ --version ; exiv2 -vV -g date -g time -g svn Sat 25 Apr 2015 21:58:44 BST ProductName: Mac OS X ProductVersion: 10.10.3 BuildVersion: 14D136 Configured with: --prefix=/Applications/Xcode.app/Contents/Developer/usr --with-gxx-include-dir=/usr/include/c++/4.2.1 Apple LLVM version 6.1.0 (clang-602.0.49) (based on LLVM 3.6.0svn) Target: x86_64-apple-darwin14.3.0 Thread model: posix exiv2 0.24 001800 (64 bit build) date=Apr 25 2015 time=21:56:27 svn=3732 have_gmtime_r=1 have_timegm=1 774 rmills@rmillsmbp:~/gnu/exiv2/trunk $
#1065. Fixing thread safety in fileProtocol. Thank You, Max for reporting this.