BxCppDev / bxdecay0

C++ port of the Decay0/GENBB fortran Monte Carlo code for the generation of standard decay or double beta decay processes for various radioactive nuclides of interest
GNU General Public License v3.0
12 stars 12 forks source link

Improve thread safety of static resources #29

Closed drbenmorgan closed 1 year ago

drbenmorgan commented 1 year ago

As reported in #28, bxdecay0 can produce segfaults when running as the primary generator of a multithreaded Geant4 application. Triaged in this use case to the use of statics in the pattern:

const thing_t& function() {
  static thing_t thing;
  if (thing.not_init()) thing.fill();
  ...
  return thing;
}

Whilst C++11 guarantees the initialization of thing to be thread safe (at least for standard types), the code following that init statement is not. For example, a vector could be initialized then filled if empty, but multiple threads could contend over this filling as some might see it as empty, others not.

The changes in the PR try to solve this through decoupling static initialisation by factoring the filling of the static objects into separate anonymous functions, e.g.

namespace {
  thing_t _init_thing() {
    thing_t tmp;
    tmp.fill();
    return tmp;
  }
}

const thing_t& function() {
  static const thing_t thing(::_init_thing());
  return thing;
}

As the initialization of the static is only called once, so is the initialization function and thus removing thread contention issues.

Note that the BinReloc functions are not modified using this pattern, but as the calls to these functions are only in bxdecay0::get_library_dir() and its init function, these should be sufficiently protected. At worst a lock_guard is needed in this one place.

All tests of bxdecay0 pass, and testing in the remage use case from #28 largely removes thread safety segfaults. There is one remaining, but this may be due to implementation details of the vertex generation there.

Also pinging @gipert for info (and optionally testing!).