3

have a code which used to run fine with boost thread under ubuntu. it's basic read only data sharing multithreading. I try to use C++11 instead of boost, very basic transition. the code compiles but have subtle bugs. crashes randomly with C++11 std thread. tried to use valgrind drd, but hard to read the debug info. Any thoughts?

==19608== Conflicting load by thread 3 at 0x00643be8 size 8
==19608==    at 0x41CEBA: std::subtract_with_carry_engine<unsigned int, 24ul, 10ul, 24ul>::operator()() (random.tcc:601)
==19608==    by 0x41CD6C: double std::generate_canonical<double, 53ul, std::subtract_with_carry_engine<unsigned int, 24ul, 10ul, 24ul> >(std::subtract_with_carry_engine<unsigned int, 24ul, 10ul, 24ul>&) (random.tcc:3475)
==19608==    by 0x41CB06: std::__detail::_Adaptor<std::subtract_with_carry_engine<unsigned int, 24ul, 10ul, 24ul>, double>::operator()() (random.h:190)
==19608==    by 0x41C877: double std::normal_distribution<double>::operator()<std::subtract_with_carry_engine<unsigned int, 24ul, 10ul, 24ul> >(std::subtract_with_carry_engine<unsigned int, 24ul, 10ul, 24ul>&, std::normal_distribution<double>::param_type const&) (random.tcc:1950)
==19608==    by 0x41C688: double std::normal_distribution<double>::operator()<std::subtract_with_carry_engine<unsigned int, 24ul, 10ul, 24ul> >(std::subtract_with_carry_engine<unsigned int, 24ul, 10ul, 24ul>&) (random.h:2196)
==19608==    by 0x41C379: nrand(double, double) (NRand.cpp:8)
==19608==    by 0x416F78: ClassDef::set_x() (LoanDef.h:407)
==19608==    by 0x4168CA: Sim(std::vector<ClassDef, std::allocator<ClassDef> >&, Assumption&, std::unordered_map<std::string, std::vector<double, std::allocator<double> >, std::hash<std::string>, std::equal_to<std::string>, std::allocator<std::pair<std::string const, std::vector<double, std::allocator<double> > > > >&, unsigned int, NextStepCalcML&, std::vector<std::vector<std::vector<double, std::allocator<double> >, std::allocator<std::vector<double, std::allocator<double> > > >, std::allocator<std::vector<std::vector<double, std::allocator<double> >, std::allocator<std::vector<double, std::allocator<double> > > > > >&, unsigned int, unsigned int) (NewSim.cpp:105)
==19608==    by 0x411A09: _ZNSt12_Bind_simpleIFPFvRSt6vectorI7LoanDefSaIS1_EER10AssumptionRSt13unordered_mapISsS0_IdSaIdEESt4hashISsESt8equal_toISsESaISt4pairIKSsS9_EEEjR14NextStepCalcMLRS0_IS0_IS9_SaIS9_EESaISN_EEjjESt17reference_wrapperIS3_EST_IS5_EST_ISI_EjST_ISK_EST_ISP_EjjEE9_M_invokeIILm0ELm1ELm2ELm3ELm4ELm5ELm6ELm7EEEEvSt12_Index_tupleIIXspT_EEE (functional:1732)
==19608==    by 0x4116AE: std::_Bind_simple<void (*()(std::reference_wrapper<std::vector<LoanDef, std::allocator<LoanDef> > >, std::reference_wrapper<Assumption>, std::reference_wrapper<std::unordered_map<std::string, std::vector<double, std::allocator<double> >, std::hash<std::string>, std::equal_to<std::string>, std::allocator<std::pair<std::string const, std::vector<double, std::allocator<double> > > > > >, unsigned int, std::reference_wrapper<NextStepCalcML>, std::reference_wrapper<std::vector<std::vector<std::vector<double, std::allocator<double> >, std::allocator<std::vector<double, std::allocator<double> > > >, std::allocator<std::vector<std::vector<double, std::allocator<double> >, std::allocator<std::vector<double, std::allocator<double> > > > > > >, unsigned int, unsigned int))(std::vector<LoanDef, std::allocator<ClassDef> >&, Assumption&, std::unordered_map<std::string, std::vector<double, std::allocator<double> >, std::hash<std::string>, std::equal_to<std::string>, std::allocator<std::pair<std::string const, std::vector<double, std::allocator<double> > > > >&, unsigned int, NextStep&, std::vector<std::vector<std::vector<double, std::allocator<double> >, std::allocator<std::vector<double, std::allocator<double> > > >, std::allocator<std::vector<std::vector<double, std::allocator<double> >, std::allocator<std::vector<double, std::allocator<double> > > > > >&, unsigned int, unsigned int)>::operator()() (functional:1720)
==19608==    by 0x411647: std::thread::_Impl<std::_Bind_simple<void (*()(std::reference_wrapper<std::vector<ClassDef, std::allocator<ClassDef> > >, std::reference_wrapper<Assumption>, std::reference_wrapper<std::unordered_map<std::string, std::vector<double, std::allocator<double> >, std::hash<std::string>, std::equal_to<std::string>, std::allocator<std::pair<std::string const, std::vector<double, std::allocator<double> > > > > >, unsigned int, std::reference_wrapper<NextStep>, std::reference_wrapper<std::vector<std::vector<std::vector<double, std::allocator<double> >, std::allocator<std::vector<double, std::allocator<double> > > >, std::allocator<std::vector<std::vector<double, std::allocator<double> >, std::allocator<std::vector<double, std::allocator<double> > > > > > >, unsigned int, unsigned int))(std::vector<LoanDef, std::allocator<LoanDef> >&, Assumption&, std::unordered_map<std::string, std::vector<double, std::allocator<double> >, std::hash<std::string>, std::equal_to<std::string>, std::allocator<std::pair<std::string const, std::vector<double, std::allocator<double> > > > >&, unsigned int, NextStep&, std::vector<std::vector<std::vector<double, std::allocator<double> >, std::allocator<std::vector<double, std::allocator<double> > > >, std::allocator<std::vector<std::vector<double, std::allocator<double> >, std::allocator<std::vector<double, std::allocator<double> > > > > >&, unsigned int, unsigned int)> >::_M_run() (thread:115)

thank you. btw, I tried to use this random head file I wrote, not sure if it's safe under multithread environment. it used to work fine with boost.

myrand.hpp

#ifndef NRAND_H
#define NRAND_H
#include <random>

double nrand(double mean = 0., double sd = 1.);
double urand(double a=0., double b=0.);

#endif

and myrand.cpp

#include "NRand.h"
using namespace std;

double nrand(double mean, double sd) {
    static random_device rd;
    static subtract_with_carry_engine<unsigned,24,10,24> e(rd());
    normal_distribution<> dist(mean, sd);
    return dist(e);
}

double urand(double a, double b) {
    static random_device rd;
    static subtract_with_carry_engine<unsigned,24,10,24> e(rd());
    uniform_real_distribution<> dist(a, b);
    return dist(e);
}

many thanks.

bbc
  • 1,061
  • 4
  • 13
  • 21
  • @Rapptz that question is undoubtful useful here, but no answer. – Walter Aug 25 '13 at 20:42
  • 2
    Your `nrand` and `urand` are in no way thread safe, and invoke undefined behavior. – Casey Aug 25 '13 at 20:44
  • should I make random device and engine non-static? then how to remove the overhead? thanks. – bbc Aug 25 '13 at 20:46
  • 1
    Another thing you may want to keep in mind is that the destructor of `std::thread` calls `std::terminate` if the thread has not been joined or detached (related: http://stackoverflow.com/questions/4508181/thread-destructors-in-c0x-vs-boost ) – MFH Aug 26 '13 at 07:36

1 Answers1

2

IMHO, your code is not threadsafe and hence should not work correctly under C++11. I think the problem is that the static variables rd and e are global variables but not protected (by mutex), so concurrent calls will race.

Presumably, you can make this code threadsafe by making those variables thread_local, but I have no experience.

Walter
  • 44,150
  • 20
  • 113
  • 196