diff --git a/Control/AthenaKernel/AthenaKernel/AlgorithmTimer.h b/Control/AthenaKernel/AthenaKernel/AlgorithmTimer.h index fff0d6dd2e2b8f3ffcbea489ada7c82989fb8e3f..72b54d21d0bc2a09557b3077e7bca6ce33a1e4c7 100644 --- a/Control/AthenaKernel/AthenaKernel/AlgorithmTimer.h +++ b/Control/AthenaKernel/AthenaKernel/AlgorithmTimer.h @@ -163,6 +163,7 @@ namespace Athena { struct sigevent m_sigevent; ///< for signal handler timer_t m_timerid; ///< timer ID callbackFct_t m_onAlarm; ///< user callback + std::atomic_bool m_active; ///< flag protecting race condition at stop private: diff --git a/Control/AthenaKernel/CMakeLists.txt b/Control/AthenaKernel/CMakeLists.txt index a2016f9bcca367fe04ec153eae642ee9daa84110..7ab072a8122bf2cbda49d1cad3585a2debee4675 100644 --- a/Control/AthenaKernel/CMakeLists.txt +++ b/Control/AthenaKernel/CMakeLists.txt @@ -157,3 +157,7 @@ atlas_add_test( ThinningCache_test atlas_add_test( getThinningCache_test SOURCES test/getThinningCache_test.cxx LINK_LIBRARIES AthenaKernel TestTools ) + + atlas_add_test( AlgorithmTimer_test + SOURCES test/AlgorithmTimer_test.cxx + LINK_LIBRARIES AthenaKernel ) diff --git a/Control/AthenaKernel/share/AlgorithmTimer_test.ref b/Control/AthenaKernel/share/AlgorithmTimer_test.ref new file mode 100644 index 0000000000000000000000000000000000000000..76983460f7f43b842604ab76921a6ea289362661 --- /dev/null +++ b/Control/AthenaKernel/share/AlgorithmTimer_test.ref @@ -0,0 +1,3 @@ +test1 +test2 +test3 diff --git a/Control/AthenaKernel/src/AlgorithmTimer.cxx b/Control/AthenaKernel/src/AlgorithmTimer.cxx index f6d3e093b814cc7a496ad17435132d20c45efec4..300fc5c3492497c60b453eda7d2bb08baaef889a 100644 --- a/Control/AthenaKernel/src/AlgorithmTimer.cxx +++ b/Control/AthenaKernel/src/AlgorithmTimer.cxx @@ -50,7 +50,8 @@ namespace Athena { void onAlarmThread(sigval_t sv) { AlgorithmTimer* me = static_cast(sv.sival_ptr); - me->m_onAlarm(); + if (me != nullptr && me->m_active) + me->m_onAlarm(); } } } @@ -113,6 +114,7 @@ AlgorithmTimer::AlgorithmTimer(unsigned int milliseconds, AlgorithmTimer::~AlgorithmTimer() { #ifndef __APPLE__ + m_active = false; timer_delete(m_timerid); #endif } @@ -128,6 +130,7 @@ void AlgorithmTimer::start(unsigned int milliseconds) spec.it_interval.tv_sec = 0; spec.it_interval.tv_nsec = 0; + m_active = true; timer_settime(m_timerid, 0, &spec, NULL); #endif } @@ -149,6 +152,7 @@ unsigned int AlgorithmTimer::stop() spec.it_interval.tv_nsec = 0; itimerspec ovalue; + m_active = false; timer_settime(m_timerid, 0, &spec, &ovalue); return 1000*ovalue.it_value.tv_sec + int(ovalue.it_value.tv_nsec/1000000); #else diff --git a/Control/AthenaKernel/test/AlgorithmTimer_test.cxx b/Control/AthenaKernel/test/AlgorithmTimer_test.cxx new file mode 100644 index 0000000000000000000000000000000000000000..db0b906e66e4a982a75610e4b38bfbb1ef3595d5 --- /dev/null +++ b/Control/AthenaKernel/test/AlgorithmTimer_test.cxx @@ -0,0 +1,84 @@ +/* + * Copyright (C) 2002-2020 CERN for the benefit of the ATLAS collaboration. + */ + +#undef NDEBUG +#include "AthenaKernel/AlgorithmTimer.h" +#include +#include +#include +#include +#include + +#define PRINT_ME {std::cout << __FUNCTION__ << std::endl;} + +using Athena::AlgorithmTimer; + +constexpr size_t numTimers = 1000; // Extreme value for testing +constexpr size_t testTimeMilliSec = 100; +constexpr size_t callbackSleepMilliSec = 1; +std::atomic_size_t counter; +std::atomic_bool allowCallbacks{false}; // Ensures no callbacks are executed after timer deletion + + +/// Create a vector of timers for testing +std::vector> createTimers(const std::size_t n, + const std::size_t t, + const AlgorithmTimer::callbackFct_t& f) { + std::vector> timers; + for (size_t i=0; i(t, f)); + return timers; +} + +/// Callback sleeping for some time and incrementing the counter +void testCallback() { + assert(allowCallbacks); + std::this_thread::sleep_for(std::chrono::milliseconds(callbackSleepMilliSec)); + ++counter; +} + +/// Test the standard timer functionality making sure N timers result in the callback +/// being executed N times within the expected time plus a fixed margin +void test1() { + PRINT_ME + counter = 0; + const auto timers = createTimers(numTimers, testTimeMilliSec, &testCallback); + allowCallbacks = true; + std::this_thread::sleep_for(std::chrono::milliseconds(testTimeMilliSec + numTimers*callbackSleepMilliSec + 50)); + allowCallbacks = false; + assert(counter == numTimers); +} + +/// Test the case when timers are deleted before the scheduled time (no callback) +void test2() { + PRINT_ME + counter = 0; + allowCallbacks = false; + auto timers = createTimers(numTimers, testTimeMilliSec, &testCallback); + std::this_thread::sleep_for(std::chrono::milliseconds(testTimeMilliSec/2)); + for (auto& ptr : timers) ptr.reset(); + // wait until the scheduled time passes to make sure no callbacks were executed + std::this_thread::sleep_for(std::chrono::milliseconds(testTimeMilliSec/2 + 50)); + assert(counter == 0); +} + +/// Test potential race condition when timers are deleted at the same time as callbacks are called +void test3() { + PRINT_ME + counter = 0; + auto timers = createTimers(numTimers, testTimeMilliSec, &testCallback); + allowCallbacks = true; + std::this_thread::sleep_for(std::chrono::milliseconds(testTimeMilliSec)); + for (auto& ptr : timers) ptr.reset(); + // ensure callback assertion fails if callback is executed after timer is deleted + allowCallbacks = false; + // no assertion on counter value, which has an undefined value here +} + +int main() { + test1(); + test2(); + test3(); + return 0; +} \ No newline at end of file