Bug #25509
Gallery threadsafety
0%
Description
First up, is gallery intended to be threadsafe? Specifically, can I call evt.getByLabel() from multiple @std::thread@s without any locking on my side? Empirically this appears to work fine if done from art, and it's worked for me in the past in gallery.
I can pretty consistently crash at art::TypeID::className() by doing that. Valgrind offers this as the first bad thing that happens.
==18911== Invalid read of size 8 ==18911== at 0x50A0BA0: art::TypeID::typeInfo() const (TypeID.cc:61) ==18911== by 0x50A13C3: art::TypeID::className[abi:cxx11]() const (TypeID.cc:74) ==18911== by 0x50A1529: art::TypeID::friendlyClassName[abi:cxx11]() const (TypeID.cc:86) ==18911== by 0x5789E85: buildBranchName (DataGetterHelper.cc:52) ==18911== by 0x5789E85: gallery::DataGetterHelper::addTypeLabelInstance(art::TypeID const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) const (DataGetterHelper.cc:444) ==18911== by 0x578A670: gallery::DataGetterHelper::getInfoForTypeLabelInstance(art::TypeID const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) const (DataGetterHelper.cc:421) ==18911== by 0x578A780: gallery::DataGetterHelper::getByLabel(std::type_info const&, art::InputTag const&) const (DataGetterHelper.cc:88) ==18911== by 0x6229051: getByLabel<std::vector<recob::Track, std::allocator<recob::Track> > > (Event.h:190)
I do see a mutex at the top of TypeID::className() which inclines me to think this is supposed to be supported.
History
#1 Updated by Christopher Backhouse 2 months ago
I should say the same code works if I make the one line change to do everything serially on one thread.
#2 Updated by Kyle Knoepfel 2 months ago
- Status changed from New to Feedback
Parts of canvas have been made thread-safe to support multi-threading in art. However, gallery was not designed with thread-safety in mind--there is much caching behind the scenes to make it efficient for single-threaded execution. If calling gallery's getByLabel from multiple threads worked for you in the past, it was entirely accidental. For now, your best bet is to ensure that calls to getByLabel are done serially.
In order for gallery to support multi-threaded execution, a feature request would have to be made, and the Scientific Computing Division would have to evaluate whether the available effort (which is small) can be spared for that endeavor.
#3 Updated by Christopher Backhouse 2 months ago
OK, that's pretty clear at least. I will go with something like this for now:
class ThreadsafeGalleryEvent
{
public:
ThreadsafeGalleryEvent(const gallery::Event* evt) : fEvt(evt)
{
}
template<class T>
bool getByLabel(const art::InputTag& tag, gallery::Handle<T>& result) const
{
std::lock_guard guard(fLock);
return fEvt->getByLabel(tag, result);
}
protected:
std::mutex fLock;
const gallery::Event* fEvt;
};
Should multiple gallery::Events pointing at different files be independent, and not needing serialization? What about pointing into the same file?