From ad6545a5de50a0eb7ccfb8eaa50f13f7011def1b Mon Sep 17 00:00:00 2001
From: Abtin Keshavarzian <abtink@google.com>
Date: Mon, 10 Jul 2023 09:25:45 -0700
Subject: [PATCH] [commissioner] simplify and fix scheduling of expiration
 timer (#9262)

This commit simplifies scheduling of `mJoinerExpirationTimer` by using
`FireAtIfEarlier()` when adding or updating a `Joiner` entry instead
of recalculating the fire time. When a `Joiner` entry is removed, we
keep the timer unchanged and let it be rescheduled when the currently
scheduled timer expires. The next expiration time is determined in
`HandleJoinerExpirationTimer()`, which ensures that all expiration
times are strictly after `now` and avoids any potential issue with
the order of comparison between `now` and `next` being
`now.GetDistantFuture()`.
---
 src/core/meshcop/commissioner.cpp | 35 +++++++------------------------
 src/core/meshcop/commissioner.hpp |  2 --
 2 files changed, 8 insertions(+), 29 deletions(-)

diff --git a/src/core/meshcop/commissioner.cpp b/src/core/meshcop/commissioner.cpp
index 27cf796d3..64dc3bd87 100644
--- a/src/core/meshcop/commissioner.cpp
+++ b/src/core/meshcop/commissioner.cpp
@@ -273,8 +273,6 @@ void Commissioner::RemoveJoinerEntry(Commissioner::Joiner &aJoiner)
         mActiveJoiner = nullptr;
     }
 
-    UpdateJoinerExpirationTimer();
-
     SendCommissionerSet();
 
     LogJoinerEntry("Removed", joinerCopy);
@@ -481,7 +479,7 @@ Error Commissioner::AddJoiner(const Mac::ExtAddress *aEui64,
 
     joiner->mExpirationTime = TimerMilli::GetNow() + Time::SecToMsec(aTimeout);
 
-    UpdateJoinerExpirationTimer();
+    mJoinerExpirationTimer.FireAtIfEarlier(joiner->mExpirationTime);
 
     SendCommissionerSet();
 
@@ -577,7 +575,7 @@ void Commissioner::RemoveJoiner(Joiner &aJoiner, uint32_t aDelay)
         if (aJoiner.mExpirationTime > newExpirationTime)
         {
             aJoiner.mExpirationTime = newExpirationTime;
-            UpdateJoinerExpirationTimer();
+            mJoinerExpirationTimer.FireAtIfEarlier(newExpirationTime);
         }
     }
     else
@@ -629,7 +627,8 @@ void Commissioner::HandleTimer(void)
 
 void Commissioner::HandleJoinerExpirationTimer(void)
 {
-    TimeMilli now = TimerMilli::GetNow();
+    TimeMilli now  = TimerMilli::GetNow();
+    TimeMilli next = now.GetDistantFuture();
 
     for (Joiner &joiner : mJoiners)
     {
@@ -643,33 +642,15 @@ void Commissioner::HandleJoinerExpirationTimer(void)
             LogDebg("removing joiner due to timeout or successfully joined");
             RemoveJoinerEntry(joiner);
         }
-    }
-
-    UpdateJoinerExpirationTimer();
-}
-
-void Commissioner::UpdateJoinerExpirationTimer(void)
-{
-    TimeMilli now  = TimerMilli::GetNow();
-    TimeMilli next = now.GetDistantFuture();
-
-    for (Joiner &joiner : mJoiners)
-    {
-        if (joiner.mType == Joiner::kTypeUnused)
+        else
         {
-            continue;
+            next = Min(joiner.mExpirationTime, next);
         }
-
-        next = Min(next, Max(now, joiner.mExpirationTime));
     }
 
-    if (next < now.GetDistantFuture())
+    if (next != now.GetDistantFuture())
     {
-        mJoinerExpirationTimer.FireAt(next);
-    }
-    else
-    {
-        mJoinerExpirationTimer.Stop();
+        mJoinerExpirationTimer.FireAtIfEarlier(next);
     }
 }
 
diff --git a/src/core/meshcop/commissioner.hpp b/src/core/meshcop/commissioner.hpp
index 8a277da23..74c910c9d 100644
--- a/src/core/meshcop/commissioner.hpp
+++ b/src/core/meshcop/commissioner.hpp
@@ -549,8 +549,6 @@ private:
     void HandleTimer(void);
     void HandleJoinerExpirationTimer(void);
 
-    void UpdateJoinerExpirationTimer(void);
-
     static void HandleMgmtCommissionerSetResponse(void                *aContext,
                                                   otMessage           *aMessage,
                                                   const otMessageInfo *aMessageInfo,