From ba844d9c071d03e42ac4b2b0f397771aabf21c26 Mon Sep 17 00:00:00 2001
From: Sebastian Reimers <hallo@studio-link.de>
Date: Fri, 12 Jan 2024 10:34:45 +0100
Subject: [PATCH] tmr: prevent race condition on cancel (#1048)

* tmr: prevent timer race condition on cancel

If `tmr_cancel` is called from two threads it's enough to cancel once.

* tmr: rename lock to list lock and improve comments
---
 include/re_tmr.h |  4 +++-
 src/tmr/tmr.c    | 18 +++++++++++++-----
 2 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/include/re_tmr.h b/include/re_tmr.h
index 17173f34f..ef9d0a6ce 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,7 +20,8 @@ struct tmrl;
 /** Defines a timer */
 struct tmr {
 	struct le le;       /**< Linked list element */
-	mtx_t *lock;        /**< Mutex lock          */
+	RE_ATOMIC bool active; /**< Timer is active  */
+	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 3c1bdc780..2d53970b0 100644
--- a/src/tmr/tmr.c
+++ b/src/tmr/tmr.c
@@ -393,10 +393,16 @@ static void tmr_startcont_dbg(struct tmr *tmr, uint64_t delay, bool syncnow,
 	if (!tmr || !tmrl)
 		return;
 
-	if (!tmr->lock || !tmr->le.list)
-		lock = tmrl->lock;
+	/* Prevent multiple cancel race conditions */
+	if (!re_atomic_acq(&tmr->active) && !th)
+		return;
+
+	re_atomic_rls_set(&tmr->active, false);
+
+	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);
 
@@ -413,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;
 	}
@@ -445,6 +451,8 @@ static void tmr_startcont_dbg(struct tmr *tmr, uint64_t delay, bool syncnow,
 		}
 	}
 
+	re_atomic_rls_set(&tmr->active, true);
+
 	mtx_unlock(lock);
 }