From f13e939cdeb9e882f1d7c871aba3e65c7c2c4a22 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Victor=20Matar=C3=A9?= Date: Wed, 4 Dec 2019 15:06:15 +0100 Subject: [PATCH] protoboard: remove check for spurious wakeups These happen occasionally because the BlackBoardOnMessageWaker is apparently slightly racey and just triggers a loop() instead of an interface/message-specific callback. No harm though except a bunch of unnecessary iterations over interfaces where nothing happened. Eventually the whole thing should however be converted to implementing a BlackBoardInterfaceListener because that would significantly reduce the complexity of the template code. --- src/libs/protoboard/blackboard_manager.cpp | 18 ++++------ src/libs/protoboard/blackboard_manager.h | 42 +++++++++++----------- 2 files changed, 26 insertions(+), 34 deletions(-) diff --git a/src/libs/protoboard/blackboard_manager.cpp b/src/libs/protoboard/blackboard_manager.cpp index 253f735cc1..cd31d3fc5f 100644 --- a/src/libs/protoboard/blackboard_manager.cpp +++ b/src/libs/protoboard/blackboard_manager.cpp @@ -79,14 +79,14 @@ void BlackboardManager::loop() { // Handle CreatePeer* messages - bool did_something = on_interface{peer_iface_, this} - .handle_msg_types(); + on_interface{peer_iface_, this} + .handle_msg_types(); // Handle sending blackboard interfaces - did_something |= pb_sender_->process_sending_interfaces(); + pb_sender_->process_sending_interfaces(); // Handle receiving blackboard interfaces while (message_handler_->pb_queue_incoming()) { @@ -108,13 +108,7 @@ BlackboardManager::loop() inc.msg->GetTypeName().c_str(), e.what()); } - - did_something = true; } - - if (!did_something) - // Thread woke up, but nothing was handled - logger->log_warn(name(), "Spurious wakeup. WTF?"); } BlackBoard * diff --git a/src/libs/protoboard/blackboard_manager.h b/src/libs/protoboard/blackboard_manager.h index 8e34f5c2dc..01490c3945 100644 --- a/src/libs/protoboard/blackboard_manager.h +++ b/src/libs/protoboard/blackboard_manager.h @@ -118,7 +118,7 @@ class AbstractProtobufSender /** Go through all interface managers, empty all blackboard message queues and send out * ProtoBuf messages accordingly. * @return whether anything was sent */ - virtual bool process_sending_interfaces() = 0; + virtual void process_sending_interfaces() = 0; /// Deferred initialization, coincides with the main thread. virtual void init() = 0; @@ -144,7 +144,7 @@ class AbstractProtobufSender * @param iface_mgr a bb_iface_manager for a specific message type * @return Whether any ProtoBuf message was sent */ template - bool operator()(const bb_iface_manager> &iface_mgr) const; + void operator()(const bb_iface_manager> &iface_mgr) const; /** Iterate through all given message types on a certain interface and * handle them individually @@ -154,7 +154,7 @@ class AbstractProtobufSender * @param iface_mgr a bb_iface_manager with a list of message type to go through * @return Whether any ProtoBuf message was sent */ template - bool + void operator()(const bb_iface_manager> &iface_mgr) const; }; }; @@ -174,10 +174,10 @@ class ProtobufSender : public AbstractProtobufSender virtual void init() override; virtual void finalize() override; - virtual bool + virtual void process_sending_interfaces() override { - return boost::fusion::any(bb_sending_interfaces_, handle_messages{this->bb_manager}); + boost::fusion::for_each(bb_sending_interfaces_, handle_messages{this->bb_manager}); } private: @@ -233,7 +233,7 @@ class BlackboardManager : public fawkes::Thread, void add_peer(fawkes::ProtobufPeerInterface *iface, long peer_id); template - bool handle_message_type(InterfaceT *iface); + void handle_message_type(InterfaceT *iface); template struct on_interface @@ -246,18 +246,19 @@ class BlackboardManager : public fawkes::Thread, } template - bool + void handle_msg_types() { - return manager->handle_message_type(iface); + manager->handle_message_type(iface); } // This template is disabled if MessageTs is {} to resolve ambiguity template - typename std::enable_if<(sizeof...(MessageTs) > 0), bool>::type + typename std::enable_if<(sizeof...(MessageTs) > 0)>::type handle_msg_types() { - return handle_msg_types() || handle_msg_types(); + handle_msg_types(); + handle_msg_types(); } }; }; @@ -286,29 +287,29 @@ ProtobufSender::finalize() } template -bool +void AbstractProtobufSender::handle_messages:: operator()(const bb_iface_manager> &pair) const { - return manager->handle_message_type(pair.interface()); + manager->handle_message_type(pair.interface()); } template -bool +void AbstractProtobufSender::handle_messages:: operator()(const bb_iface_manager> &iface_mgr) const { - return BlackboardManager::on_interface{iface_mgr.interface(), manager} - .template handle_msg_types() - || manager->handle_message_type(iface_mgr.interface()); + BlackboardManager::on_interface{iface_mgr.interface(), manager} + .template handle_msg_types(); + + manager->handle_message_type(iface_mgr.interface()); } template -bool +void BlackboardManager::handle_message_type(InterfaceT *iface) { if (!iface->msgq_empty()) { - bool rv = false; while (MessageT *msg = iface->msgq_first_safe(msg)) { try { handle_message(iface, msg); @@ -318,11 +319,8 @@ BlackboardManager::handle_message_type(InterfaceT *iface) name(), "Exception handling %s on %s: %s", msg->type(), iface->uid(), e.what()); } iface->msgq_pop(); - rv = true; } - return rv; - } else - return false; + } } } // namespace protoboard