From 2887c0fbb14d266cf3a43997b12c8d7691d0d730 Mon Sep 17 00:00:00 2001
From: Jovan Bunjevacki <bjovan@gmail.com>
Date: Sat, 9 May 2020 00:51:36 +0200
Subject: [PATCH] Problem: Usage of invalidated iterator of _timers container
 in zmq::poller_base_t::execute_timers (). Solution: Safe iteration through
 _timers container, valid only for std::multimap (currently it is).

---
 src/poller_base.cpp | 41 ++++++++++++++++++++++++++---------------
 1 file changed, 26 insertions(+), 15 deletions(-)

diff --git a/src/poller_base.cpp b/src/poller_base.cpp
index 014b2779..d4cc07fa 100644
--- a/src/poller_base.cpp
+++ b/src/poller_base.cpp
@@ -68,8 +68,14 @@ void zmq::poller_base_t::cancel_timer (i_poll_events *sink_, int id_)
             return;
         }
 
-    //  Timer not found.
-    zmq_assert (false);
+    //  We should generally never get here. Calling 'cancel_timer ()' on
+    //  an already expired or canceled timer (or even worse - on a timer which
+    //  never existed, supplying bad sink_ and/or id_ values) does not make any
+    //  sense.
+    //  But in some edge cases this might happen. As described in issue #3645
+    //  `timer_event ()` call from `execute_timers ()` might call `cancel_timer ()`
+    //  on already canceled (deleted) timer.
+    //  As soon as that is resolved an 'assert (false)' should be put here.
 }
 
 uint64_t zmq::poller_base_t::execute_timers ()
@@ -81,26 +87,31 @@ uint64_t zmq::poller_base_t::execute_timers ()
     //  Get the current time.
     const uint64_t current = _clock.now_ms ();
 
-    //   Execute the timers that are already due.
-    const timers_t::iterator begin = _timers.begin ();
-    const timers_t::iterator end = _timers.end ();
+    //  Execute the timers that are already due.
     uint64_t res = 0;
-    timers_t::iterator it = begin;
-    for (; it != end; ++it) {
-        //  If we have to wait to execute the item, same will be true about
-        //  all the following items (multimap is sorted). Thus we can stop
-        //  checking the subsequent timers.
+    timer_info_t timer_temp;
+    timers_t::iterator it;
+
+    do {
+        it = _timers.begin ();
+
+        //  If we have to wait to execute the item, same will be true for
+        //  all the following items because multimap is sorted. Thus we can
+        //  stop checking the subsequent timers.
         if (it->first > current) {
             res = it->first - current;
             break;
         }
 
-        //  Trigger the timer.
-        it->second.sink->timer_event (it->second.id);
-    }
+        //  Save and remove the timer because timer_event() call might delete
+        //  exactly this timer and then the iterator will be invalid.
+        timer_temp = it->second;
+        _timers.erase (it);
 
-    //  Remove them from the list of active timers.
-    _timers.erase (begin, it);
+        //  Trigger the timer.
+        timer_temp.sink->timer_event (timer_temp.id);
+
+    } while (!_timers.empty ());
 
     //  Return the time to wait for the next timer (at least 1ms), or 0, if
     //  there are no more timers.