fix: debounce_timeout compare different iterators

Function get_subscriber() returns a copy of the set (not a reference). A new copy is returned on every call. Therefore, the comparison happens between iterators of two different copies of the set - which will never be equal. Generally the get_subscriber should be refactored to return a reference instead of copy.
COVESA · Jul 12, 2024 · 18a2462 · 18a2462
1 parent 6c0e9db
commit 18a2462
Showing 1 changed file with 40 additions and 25 deletions.
diff --git a/implementation/routing/src/routing_manager_base.cpp b/implementation/routing/src/routing_manager_base.cpp
@@ -50,41 +50,56 @@ routing_manager_base::routing_manager_base(routing_manager_host *_host) :
     }
 }
 
-void routing_manager_base::debounce_timeout_update_cbk(const boost::system::error_code &_error, const std::shared_ptr<vsomeip_v3::event> &_event, client_t _client, const std::shared_ptr<debounce_filter_impl_t> &_filter) {
+void routing_manager_base::debounce_timeout_update_cbk(const boost::system::error_code &_error,
+                                                       const std::shared_ptr<vsomeip_v3::event> &_event,
+                                                       client_t _client,
+                                                       const std::shared_ptr<debounce_filter_impl_t> &_filter) {
     if (!_error) {
         std::lock_guard<std::mutex> its_lock(debounce_mutex_);
-        if (_event && (_event->get_subscribers().find(_client) != _event->get_subscribers().end()) && _filter) {
-            bool is_elapsed{false};
-            auto its_current = std::chrono::steady_clock::now();
-
-            int64_t elapsed = std::chrono::duration_cast<std::chrono::milliseconds>(
-                    its_current - _filter->last_forwarded_).count();
-            is_elapsed = (_filter->last_forwarded_ == std::chrono::steady_clock::time_point::max()
-                    || elapsed >= _filter->interval_);
-
-            auto debounce_client = debounce_clients_.begin();
-            auto event_id = std::get<3>(debounce_client->second);
-            bool has_update = std::get<1>(debounce_client->second);
-            if (is_elapsed) {
-                if (std::get<0>(debounce_client->second) == _client && has_update) {
-                    _event->notify_one(_client, false);
-                    has_update = false;
+        if (_event) {
+            std::set<client_t> its_subscribers;
+            its_subscribers = _event->get_subscribers();
+
+            if ((its_subscribers.find(_client) != its_subscribers.end()) && _filter) {
+                bool is_elapsed{false};
+                auto its_current = std::chrono::steady_clock::now();
+
+                int64_t elapsed =
+                    std::chrono::duration_cast<std::chrono::milliseconds>(its_current - _filter->last_forwarded_)
+                        .count();
+                is_elapsed = (_filter->last_forwarded_ == std::chrono::steady_clock::time_point::max() ||
+                              elapsed >= _filter->interval_);
+
+                auto debounce_client = debounce_clients_.begin();
+                auto event_id = std::get<3>(debounce_client->second);
+                bool has_update = std::get<1>(debounce_client->second);
+                if (is_elapsed) {
+                    if (std::get<0>(debounce_client->second) == _client && has_update) {
+                        _event->notify_one(_client, false);
+                        has_update = false;
+                    }
+                    elapsed = 0;
                 }
-                elapsed = 0;
-            }
 
-            debounce_clients_.erase(debounce_client);
+                debounce_clients_.erase(debounce_client);
 
-            auto timeout = std::chrono::steady_clock::now() + std::chrono::milliseconds(_filter->interval_ - elapsed);
-            debounce_clients_.emplace(timeout, std::make_tuple(_client, has_update, std::bind(&routing_manager_base::debounce_timeout_update_cbk, shared_from_this(),
-                            std::placeholders::_1, _event, _client, _filter), event_id));
+                auto timeout =
+                    std::chrono::steady_clock::now() + std::chrono::milliseconds(_filter->interval_ - elapsed);
+                debounce_clients_.emplace(
+                    timeout,
+                    std::make_tuple(_client, has_update,
+                                    std::bind(&routing_manager_base::debounce_timeout_update_cbk, shared_from_this(),
+                                              std::placeholders::_1, _event, _client, _filter),
+                                    event_id));
+            }
         }
 
         if (debounce_clients_.size() > 0) {
-            debounce_timer.expires_from_now(std::chrono::duration_cast<std::chrono::milliseconds>(debounce_clients_.begin()->first - std::chrono::steady_clock::now()));
+            debounce_timer.expires_from_now(std::chrono::duration_cast<std::chrono::milliseconds>(
+                debounce_clients_.begin()->first - std::chrono::steady_clock::now()));
             debounce_timer.async_wait(std::get<2>(debounce_clients_.begin()->second));
         }
-    } 
+    }
 }
 
 void routing_manager_base::register_debounce(const std::shared_ptr<debounce_filter_impl_t> &_filter, client_t _client, const  std::shared_ptr<vsomeip_v3::event> &_event) {