From 5132339b7bfe0af8422026dccad7534f784ff548 Mon Sep 17 00:00:00 2001 From: Niclas Antti Date: Tue, 25 Sep 2018 15:47:41 +0300 Subject: [PATCH] Fix race condition. Change a counter (s_next_delayed_call_id) in Worker from class static to class member to avoid race conditions on the variable. --- maxutils/maxbase/include/maxbase/worker.hh | 38 +++++++++++++--------- maxutils/maxbase/src/worker.cc | 4 +-- 2 files changed, 23 insertions(+), 19 deletions(-) diff --git a/maxutils/maxbase/include/maxbase/worker.hh b/maxutils/maxbase/include/maxbase/worker.hh index 68ad50d7c..9157a7885 100644 --- a/maxutils/maxbase/include/maxbase/worker.hh +++ b/maxutils/maxbase/include/maxbase/worker.hh @@ -573,10 +573,9 @@ public: * case the return value is ignored and the function will not * be called again. */ - uint32_t delayed_call(int32_t delay, - bool (* pFunction)(Worker::Call::action_t action)) + uint32_t delayed_call(int32_t delay, bool (* pFunction)(Worker::Call::action_t action)) { - return add_delayed_call(new DelayedCallFunctionVoid(delay, pFunction)); + return add_delayed_call(new DelayedCallFunctionVoid(delay, next_delayed_call_id(), pFunction)); } /** @@ -604,7 +603,7 @@ public: bool (* pFunction)(Worker::Call::action_t action, D data), D data) { - return add_delayed_call(new DelayedCallFunction(delay, pFunction, data)); + return add_delayed_call(new DelayedCallFunction(delay, next_delayed_call_id(), pFunction, data)); } /** @@ -631,7 +630,7 @@ public: bool (T::* pMethod)(Worker::Call::action_t action), T* pT) { - return add_delayed_call(new DelayedCallMethodVoid(delay, pMethod, pT)); + return add_delayed_call(new DelayedCallMethodVoid(delay, next_delayed_call_id(), pMethod, pT)); } /** @@ -660,7 +659,11 @@ public: T* pT, D data) { - return add_delayed_call(new DelayedCallMethod(delay, pMethod, pT, data)); + return add_delayed_call(new DelayedCallMethod(delay, + next_delayed_call_id(), + pMethod, + pT, + data)); } /** @@ -734,12 +737,12 @@ private: class DelayedCall; friend class DelayedCall; - static uint32_t next_delayed_call_id() + uint32_t next_delayed_call_id() { // Called in single-thread context. Wrapping does not matter // as it is unlikely there would be 4 billion pending delayed // calls. - return ++s_next_delayed_call_id; + return ++m_next_delayed_call_id; } class DelayedCall @@ -779,8 +782,8 @@ private: } protected: - DelayedCall(int32_t delay) - : m_id(Worker::next_delayed_call_id()) + DelayedCall(int32_t delay, int32_t id) + : m_id(id) , m_delay(delay) , m_at(get_at(delay)) { @@ -815,9 +818,10 @@ private: public: DelayedCallFunction(int32_t delay, + int32_t id, bool (*pFunction)(Worker::Call::action_t action, D data), D data) - : DelayedCall(delay) + : DelayedCall(delay, id) , m_pFunction(pFunction) , m_data(data) { @@ -842,8 +846,9 @@ private: public: DelayedCallFunctionVoid(int32_t delay, + int32_t id, bool (*pFunction)(Worker::Call::action_t action)) - : DelayedCall(delay) + : DelayedCall(delay, id) , m_pFunction(pFunction) { } @@ -866,10 +871,11 @@ private: public: DelayedCallMethod(int32_t delay, + int32_t id, bool (T::* pMethod)(Worker::Call::action_t action, D data), T* pT, D data) - : DelayedCall(delay) + : DelayedCall(delay, id) , m_pMethod(pMethod) , m_pT(pT) , m_data(data) @@ -896,9 +902,10 @@ private: public: DelayedCallMethodVoid(int32_t delay, + int32_t id, bool (T::* pMethod)(Worker::Call::action_t), T* pT) - : DelayedCall(delay) + : DelayedCall(delay, id) , m_pMethod(pMethod) , m_pT(pT) { @@ -925,7 +932,6 @@ private: void poll_waitevents(); void tick(); - private: class LaterAt : public std::binary_function { @@ -956,6 +962,6 @@ private: DelayedCallsByTime m_sorted_calls; /*< Current delayed calls sorted by time. */ DelayedCallsById m_calls; /*< Current delayed calls indexed by id. */ - static uint32_t s_next_delayed_call_id; /*< The next delayed call id. */ + int32_t m_next_delayed_call_id; /*< The next delayed call id. */ }; } diff --git a/maxutils/maxbase/src/worker.cc b/maxutils/maxbase/src/worker.cc index 2ff5a63d2..64297b139 100644 --- a/maxutils/maxbase/src/worker.cc +++ b/maxutils/maxbase/src/worker.cc @@ -278,9 +278,6 @@ int create_epoll_instance() } } -// static -uint32_t Worker::s_next_delayed_call_id = 1; - Worker::Worker(int max_events) : m_epoll_fd(create_epoll_instance()) , m_state(STOPPED) @@ -292,6 +289,7 @@ Worker::Worker(int max_events) , m_nCurrent_descriptors(0) , m_nTotal_descriptors(0) , m_pTimer(new PrivateTimer(this, this, &Worker::tick)) + , m_next_delayed_call_id{1} { mxb_assert(max_events > 0);