diff --git a/src/modules/uORB/uORBDeviceMaster.cpp b/src/modules/uORB/uORBDeviceMaster.cpp index 6bce477de6c22172c0cd37e6744f711b9b286cd7..6ccdc4d016274c443cc9023181671eec755e6d4c 100644 --- a/src/modules/uORB/uORBDeviceMaster.cpp +++ b/src/modules/uORB/uORBDeviceMaster.cpp @@ -161,22 +161,38 @@ void uORB::DeviceMaster::printStatistics(bool reset) PX4_INFO("TOPIC, NR LOST MSGS"); bool had_print = false; + /* Add all nodes to a list while locked, and then print them in unlocked state, to avoid potential + * dead-locks (where printing blocks) */ lock(); + DeviceNodeStatisticsData *first_node = nullptr; + DeviceNodeStatisticsData *cur_node = nullptr; + size_t max_topic_name_length = 0; + int num_topics = 0; + int ret = addNewDeviceNodes(&first_node, num_topics, max_topic_name_length, nullptr, 0); + unlock(); - for (const auto &node : _node_list) { - if (node->print_statistics(reset)) { + if (ret != 0) { + PX4_ERR("addNewDeviceNodes failed (%i)", ret); + } + + cur_node = first_node; + + while (cur_node) { + if (cur_node->node->print_statistics(reset)) { had_print = true; } - } - unlock(); + DeviceNodeStatisticsData *prev = cur_node; + cur_node = cur_node->next; + delete prev; + } if (!had_print) { PX4_INFO("No lost messages"); } } -void uORB::DeviceMaster::addNewDeviceNodes(DeviceNodeStatisticsData **first_node, int &num_topics, +int uORB::DeviceMaster::addNewDeviceNodes(DeviceNodeStatisticsData **first_node, int &num_topics, size_t &max_topic_name_length, char **topic_filter, int num_filters) { DeviceNodeStatisticsData *cur_node = nullptr; @@ -227,8 +243,7 @@ void uORB::DeviceMaster::addNewDeviceNodes(DeviceNodeStatisticsData **first_node } if (!last_node) { - PX4_ERR("mem alloc failed"); - break; + return -ENOMEM; } last_node->node = node; @@ -242,6 +257,8 @@ void uORB::DeviceMaster::addNewDeviceNodes(DeviceNodeStatisticsData **first_node last_node->last_lost_msg_count = last_node->node->lost_message_count(); last_node->last_pub_msg_count = last_node->node->published_message_count(); } + + return 0; } #define CLEAR_LINE "\033[K" @@ -284,11 +301,15 @@ void uORB::DeviceMaster::showTop(char **topic_filter, int num_filters) DeviceNodeStatisticsData *cur_node = nullptr; size_t max_topic_name_length = 0; int num_topics = 0; - addNewDeviceNodes(&first_node, num_topics, max_topic_name_length, topic_filter, num_filters); + int ret = addNewDeviceNodes(&first_node, num_topics, max_topic_name_length, topic_filter, num_filters); /* a DeviceNode is never deleted, so it's save to unlock here and still access the DeviceNodes */ unlock(); + if (ret != 0) { + PX4_ERR("addNewDeviceNodes failed (%i)", ret); + } + #ifdef __PX4_QURT //QuRT has no poll() only_once = true; #else @@ -310,7 +331,7 @@ void uORB::DeviceMaster::showTop(char **topic_filter, int num_filters) for (int k = 0; k < 5; k++) { char c; - int ret = ::poll(&fds, 1, 0); //just want to check if there is new data available + ret = ::poll(&fds, 1, 0); //just want to check if there is new data available if (ret > 0) { @@ -365,8 +386,13 @@ void uORB::DeviceMaster::showTop(char **topic_filter, int num_filters) } lock(); - addNewDeviceNodes(&first_node, num_topics, max_topic_name_length, topic_filter, num_filters); + ret = addNewDeviceNodes(&first_node, num_topics, max_topic_name_length, topic_filter, num_filters); unlock(); + + if (ret != 0) { + PX4_ERR("addNewDeviceNodes failed (%i)", ret); + } + } if (only_once) { diff --git a/src/modules/uORB/uORBDeviceMaster.hpp b/src/modules/uORB/uORBDeviceMaster.hpp index 242201f27553f98820e3487aed29cff467c244f5..60c9c0d62880c388c06af6254679c6fa0077a1a0 100644 --- a/src/modules/uORB/uORBDeviceMaster.hpp +++ b/src/modules/uORB/uORBDeviceMaster.hpp @@ -98,8 +98,8 @@ private: DeviceNodeStatisticsData *next = nullptr; }; - void addNewDeviceNodes(DeviceNodeStatisticsData **first_node, int &num_topics, size_t &max_topic_name_length, - char **topic_filter, int num_filters); + int addNewDeviceNodes(DeviceNodeStatisticsData **first_node, int &num_topics, size_t &max_topic_name_length, + char **topic_filter, int num_filters); friend class uORB::Manager;