mirror of
https://gitee.com/mirrors_PX4/PX4-Autopilot.git
synced 2026-10-28 00:43:34 +08:00
uorb: avoid printf while DeviceMaster is locked
This fixes a potential dead-lock when 'uorb status' was used via MAVLink shell. The dead-lock chain is: DeviceMaster::lock() -> printf -> output to a pipe, which blocks until a reader reads the data. In that case it's mavlink. If mavlink makes a call that requires DeviceMaster::lock() (such as orb_exists), it dead-locks. This patch moves all printf's out of the locked state.
This commit is contained in:
@@ -161,22 +161,38 @@ void uORB::DeviceMaster::printStatistics(bool reset)
|
|||||||
PX4_INFO("TOPIC, NR LOST MSGS");
|
PX4_INFO("TOPIC, NR LOST MSGS");
|
||||||
bool had_print = false;
|
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();
|
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 (ret != 0) {
|
||||||
if (node->print_statistics(reset)) {
|
PX4_ERR("addNewDeviceNodes failed (%i)", ret);
|
||||||
had_print = true;
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
unlock();
|
cur_node = first_node;
|
||||||
|
|
||||||
|
while (cur_node) {
|
||||||
|
if (cur_node->node->print_statistics(reset)) {
|
||||||
|
had_print = true;
|
||||||
|
}
|
||||||
|
|
||||||
|
DeviceNodeStatisticsData *prev = cur_node;
|
||||||
|
cur_node = cur_node->next;
|
||||||
|
delete prev;
|
||||||
|
}
|
||||||
|
|
||||||
if (!had_print) {
|
if (!had_print) {
|
||||||
PX4_INFO("No lost messages");
|
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)
|
size_t &max_topic_name_length, char **topic_filter, int num_filters)
|
||||||
{
|
{
|
||||||
DeviceNodeStatisticsData *cur_node = nullptr;
|
DeviceNodeStatisticsData *cur_node = nullptr;
|
||||||
@@ -227,8 +243,7 @@ void uORB::DeviceMaster::addNewDeviceNodes(DeviceNodeStatisticsData **first_node
|
|||||||
}
|
}
|
||||||
|
|
||||||
if (!last_node) {
|
if (!last_node) {
|
||||||
PX4_ERR("mem alloc failed");
|
return -ENOMEM;
|
||||||
break;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
last_node->node = node;
|
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_lost_msg_count = last_node->node->lost_message_count();
|
||||||
last_node->last_pub_msg_count = last_node->node->published_message_count();
|
last_node->last_pub_msg_count = last_node->node->published_message_count();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
return 0;
|
||||||
}
|
}
|
||||||
|
|
||||||
#define CLEAR_LINE "\033[K"
|
#define CLEAR_LINE "\033[K"
|
||||||
@@ -284,11 +301,15 @@ void uORB::DeviceMaster::showTop(char **topic_filter, int num_filters)
|
|||||||
DeviceNodeStatisticsData *cur_node = nullptr;
|
DeviceNodeStatisticsData *cur_node = nullptr;
|
||||||
size_t max_topic_name_length = 0;
|
size_t max_topic_name_length = 0;
|
||||||
int num_topics = 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 */
|
/* a DeviceNode is never deleted, so it's save to unlock here and still access the DeviceNodes */
|
||||||
unlock();
|
unlock();
|
||||||
|
|
||||||
|
if (ret != 0) {
|
||||||
|
PX4_ERR("addNewDeviceNodes failed (%i)", ret);
|
||||||
|
}
|
||||||
|
|
||||||
#ifdef __PX4_QURT //QuRT has no poll()
|
#ifdef __PX4_QURT //QuRT has no poll()
|
||||||
only_once = true;
|
only_once = true;
|
||||||
#else
|
#else
|
||||||
@@ -310,7 +331,7 @@ void uORB::DeviceMaster::showTop(char **topic_filter, int num_filters)
|
|||||||
for (int k = 0; k < 5; k++) {
|
for (int k = 0; k < 5; k++) {
|
||||||
char c;
|
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) {
|
if (ret > 0) {
|
||||||
|
|
||||||
@@ -365,8 +386,13 @@ void uORB::DeviceMaster::showTop(char **topic_filter, int num_filters)
|
|||||||
}
|
}
|
||||||
|
|
||||||
lock();
|
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();
|
unlock();
|
||||||
|
|
||||||
|
if (ret != 0) {
|
||||||
|
PX4_ERR("addNewDeviceNodes failed (%i)", ret);
|
||||||
|
}
|
||||||
|
|
||||||
}
|
}
|
||||||
|
|
||||||
if (only_once) {
|
if (only_once) {
|
||||||
|
|||||||
@@ -98,8 +98,8 @@ private:
|
|||||||
DeviceNodeStatisticsData *next = nullptr;
|
DeviceNodeStatisticsData *next = nullptr;
|
||||||
};
|
};
|
||||||
|
|
||||||
void addNewDeviceNodes(DeviceNodeStatisticsData **first_node, int &num_topics, size_t &max_topic_name_length,
|
int addNewDeviceNodes(DeviceNodeStatisticsData **first_node, int &num_topics, size_t &max_topic_name_length,
|
||||||
char **topic_filter, int num_filters);
|
char **topic_filter, int num_filters);
|
||||||
|
|
||||||
friend class uORB::Manager;
|
friend class uORB::Manager;
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user