From f989f45a8fcbbac37ca464a78d8eb774b5dd8dd2 Mon Sep 17 00:00:00 2001 From: Sebastian Reimers Date: Fri, 12 Jan 2024 09:48:58 +0100 Subject: [PATCH 1/2] tmr: prevent timer race condition on cancel If `tmr_cancel` is called from two threads it's enough to cancel once. --- include/re_tmr.h | 2 ++ src/tmr/tmr.c | 7 +++++++ 2 files changed, 9 insertions(+) diff --git a/include/re_tmr.h b/include/re_tmr.h index 17173f34f..04ebca05e 100644 --- a/include/re_tmr.h +++ b/include/re_tmr.h @@ -6,6 +6,7 @@ #include "re_thread.h" +#include "re_atomic.h" /** * Defines the timeout handler @@ -19,6 +20,7 @@ struct tmrl; /** Defines a timer */ struct tmr { struct le le; /**< Linked list element */ + RE_ATOMIC bool active; /**< Timer is active */ mtx_t *lock; /**< Mutex lock */ tmr_h *th; /**< Timeout handler */ void *arg; /**< Handler argument */ diff --git a/src/tmr/tmr.c b/src/tmr/tmr.c index 3c1bdc780..3684735c9 100644 --- a/src/tmr/tmr.c +++ b/src/tmr/tmr.c @@ -393,6 +393,11 @@ static void tmr_startcont_dbg(struct tmr *tmr, uint64_t delay, bool syncnow, if (!tmr || !tmrl) return; + if (!re_atomic_acq(&tmr->active) && !th) + return; + + re_atomic_rls_set(&tmr->active, false); + if (!tmr->lock || !tmr->le.list) lock = tmrl->lock; else @@ -445,6 +450,8 @@ static void tmr_startcont_dbg(struct tmr *tmr, uint64_t delay, bool syncnow, } } + re_atomic_rls_set(&tmr->active, true); + mtx_unlock(lock); } From d47f72b7cc43dbed6f75df8187a9d92510a62790 Mon Sep 17 00:00:00 2001 From: Sebastian Reimers Date: Fri, 12 Jan 2024 10:07:27 +0100 Subject: [PATCH 2/2] tmr: rename lock to list lock and improve comments --- include/re_tmr.h | 2 +- src/tmr/tmr.c | 11 ++++++----- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/include/re_tmr.h b/include/re_tmr.h index 04ebca05e..ef9d0a6ce 100644 --- a/include/re_tmr.h +++ b/include/re_tmr.h @@ -21,7 +21,7 @@ struct tmrl; struct tmr { struct le le; /**< Linked list element */ RE_ATOMIC bool active; /**< Timer is active */ - mtx_t *lock; /**< Mutex lock */ + mtx_t *llock; /**< List Mutex lock */ tmr_h *th; /**< Timeout handler */ void *arg; /**< Handler argument */ uint64_t jfs; /**< Jiffies for timeout */ diff --git a/src/tmr/tmr.c b/src/tmr/tmr.c index 3684735c9..2d53970b0 100644 --- a/src/tmr/tmr.c +++ b/src/tmr/tmr.c @@ -393,15 +393,16 @@ static void tmr_startcont_dbg(struct tmr *tmr, uint64_t delay, bool syncnow, if (!tmr || !tmrl) return; + /* Prevent multiple cancel race conditions */ if (!re_atomic_acq(&tmr->active) && !th) return; re_atomic_rls_set(&tmr->active, false); - if (!tmr->lock || !tmr->le.list) - lock = tmrl->lock; + if (!tmr->llock || !tmr->le.list) + lock = tmrl->lock; /* use current list lock */ else - lock = tmr->lock; /* use old lock for unlinking */ + lock = tmr->llock; /* use old list lock for unlinking */ mtx_lock(lock); @@ -418,10 +419,10 @@ static void tmr_startcont_dbg(struct tmr *tmr, uint64_t delay, bool syncnow, tmr->arg = arg; tmr->file = file; tmr->line = line; - tmr->lock = tmrl->lock; + tmr->llock = tmrl->lock; if (!th) { - tmr->lock = NULL; + tmr->llock = NULL; mtx_unlock(lock); return; }