From ce2d4017f11b5afa9ddda1fa7e81d4c710b1c367 Mon Sep 17 00:00:00 2001 From: Stephan Bergmann Date: Wed, 19 Jun 2019 18:20:35 +0200 Subject: Lock comphelper::rng internals for multi-threaded access With `--convert-to pdf xlsx/tdf116206-1.xlsx` with xlsx/tdf116206-1.xlsx as obtained by bin/get-bugzilla-attachments-by-mimetype (i.e., the attachment "Example file with lots of random data" at ), my ASan+UBSan build will eventually fail with > .../include/c++/10.0.0/bits/random.tcc:461:25: runtime error: index 624 out of bounds for type 'unsigned long [624]' > #0 in std::mersenne_twister_engine::operator()() at .../include/c++/10.0.0/bits/random.tcc:461:25 > #1 in double std::generate_canonical >(std::mersenne_twister_engine&) at .../include/c++/10.0.0/bits/random.tcc:3336:23 > #2 in std::__detail::_Adaptor, double>::operator()() at .../include/c++/10.0.0/bits/random.h:179:11 > #3 in double std::uniform_real_distribution::operator() >(std::mersenne_twister_engine&, std::uniform_real_distribution::param_type const&) at .../include/c++/10.0.0/bits/random.h:1857:12 > #4 in double std::uniform_real_distribution::operator() >(std::mersenne_twister_engine&) at .../include/c++/10.0.0/bits/random.h:1848:24 > #5 in comphelper::rng::uniform_real_distribution(double, double) at comphelper/source/misc/random.cxx:104:12 > #6 in ScInterpreter::ScRandom() at sc/source/core/tool/interpr1.cxx:1768:21 > #7 in ScInterpreter::Interpret() at sc/source/core/tool/interpr4.cxx:4026:43 > #8 in ScFormulaCell::InterpretTail(ScInterpreterContext&, ScFormulaCell::ScInterpretTailParameter) at sc/source/core/data/formulacell.cxx:1905:23 > #9 in ScColumn::CalculateInThread(ScInterpreterContext&, int, unsigned long, unsigned int, unsigned int) at sc/source/core/data/column2.cxx:2964:15 > #10 in ScTable::CalculateInColumnInThread(ScInterpreterContext&, short, int, unsigned long, unsigned int, unsigned int) at sc/source/core/data/table1.cxx:2476:16 > #11 in ScDocument::CalculateInColumnInThread(ScInterpreterContext&, ScAddress const&, unsigned long, unsigned int, unsigned int) at sc/source/core/data/documen8.cxx:422:11 > #12 in ScFormulaCell::InterpretFormulaGroupThreading(sc::FormulaLogger::GroupScope&, bool&, bool&, int, int)::Executor::doWork() at sc/source/core/data/formulacell.cxx:4714:29 > #13 in comphelper::ThreadTask::exec() at comphelper/source/misc/threadpool.cxx:279:9 > #14 in comphelper::ThreadPool::ThreadWorker::execute() at comphelper/source/misc/threadpool.cxx:83:24 > #15 in salhelper::Thread::run() at salhelper/source/thread.cxx:40:9 [...] suggesting that there is racy concurrent access to the internals of singleton std::mt19937 comphelper::rng::RandomNumberGenerator::global_rng. Change-Id: I8209b3903918c567fc832abbb84c8fbcc08e444b Reviewed-on: https://gerrit.libreoffice.org/74368 Tested-by: Jenkins Reviewed-by: Stephan Bergmann --- comphelper/source/misc/random.cxx | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) (limited to 'comphelper') diff --git a/comphelper/source/misc/random.cxx b/comphelper/source/misc/random.cxx index bfb4de15aee5..ddc970efe321 100644 --- a/comphelper/source/misc/random.cxx +++ b/comphelper/source/misc/random.cxx @@ -15,6 +15,7 @@ #include #include #include +#include #include #include #if defined HAVE_VALGRIND_HEADERS @@ -39,6 +40,7 @@ namespace rng struct RandomNumberGenerator { + std::mutex mutex; STD_RNG_ALGO global_rng; RandomNumberGenerator() { @@ -79,21 +81,27 @@ class theRandomNumberGenerator : public rtl::Static dist(a, b); - return dist(theRandomNumberGenerator::get().global_rng); + auto & gen = theRandomNumberGenerator::get(); + std::scoped_lock g(gen.mutex); + return dist(gen.global_rng); } // uniform ints [a,b] distribution unsigned int uniform_uint_distribution(unsigned int a, unsigned int b) { std::uniform_int_distribution dist(a, b); - return dist(theRandomNumberGenerator::get().global_rng); + auto & gen = theRandomNumberGenerator::get(); + std::scoped_lock g(gen.mutex); + return dist(gen.global_rng); } // uniform size_t [a,b] distribution size_t uniform_size_distribution(size_t a, size_t b) { std::uniform_int_distribution dist(a, b); - return dist(theRandomNumberGenerator::get().global_rng); + auto & gen = theRandomNumberGenerator::get(); + std::scoped_lock g(gen.mutex); + return dist(gen.global_rng); } // uniform size_t [a,b) distribution @@ -101,7 +109,9 @@ double uniform_real_distribution(double a, double b) { assert(a < b); std::uniform_real_distribution dist(a, b); - return dist(theRandomNumberGenerator::get().global_rng); + auto & gen = theRandomNumberGenerator::get(); + std::scoped_lock g(gen.mutex); + return dist(gen.global_rng); } } // namespace -- cgit