From e8937a98a507cabb7e9b72963090b44617dccfba Mon Sep 17 00:00:00 2001 From: Julian Oes Date: Wed, 18 Feb 2026 20:22:10 +1300 Subject: [PATCH] dataman: fix TSAN issues --- src/modules/dataman/dataman.cpp | 35 ++++++++++++++++++++------------- 1 file changed, 21 insertions(+), 14 deletions(-) diff --git a/src/modules/dataman/dataman.cpp b/src/modules/dataman/dataman.cpp index ae5d294fda..7d81651385 100644 --- a/src/modules/dataman/dataman.cpp +++ b/src/modules/dataman/dataman.cpp @@ -57,6 +57,8 @@ #include #include +#include + #include "dataman.h" __BEGIN_DECLS @@ -126,7 +128,7 @@ static struct { uint8_t *data_end; } ram; }; - bool running; + px4::atomic_bool running{false}; bool silence = false; } dm_operations_data; @@ -172,13 +174,14 @@ static enum { static px4_sem_t g_init_sema; -static bool g_task_should_exit; /**< if true, dataman task should exit */ +static px4::atomic_bool g_task_should_exit{false}; /**< if true, dataman task should exit */ +static px4_task_t g_task_id = -1; /**< dataman task handle for join */ /* Work queue management functions */ static bool is_running() { - return dm_operations_data.running; + return dm_operations_data.running.load(); } /* Calculate the offset in file of specific item */ @@ -607,7 +610,7 @@ _file_initialize(unsigned max_offset) g_dm_ops->write(DM_KEY_SAFE_POINTS_STATE, 0, reinterpret_cast(&stats), sizeof(mission_stats_entry_s)); } - dm_operations_data.running = true; + dm_operations_data.running.store(true); return 0; } @@ -627,7 +630,7 @@ _ram_initialize(unsigned max_offset) memset(dm_operations_data.ram.data, 0, max_offset); dm_operations_data.ram.data_end = &dm_operations_data.ram.data[max_offset - 1]; - dm_operations_data.running = true; + dm_operations_data.running.store(true); return 0; } @@ -637,7 +640,7 @@ static void _file_shutdown() { close(dm_operations_data.file.fd); - dm_operations_data.running = false; + dm_operations_data.running.store(false); } #endif @@ -645,7 +648,7 @@ static void _ram_shutdown() { free(dm_operations_data.ram.data); - dm_operations_data.running = false; + dm_operations_data.running.store(false); } static int @@ -683,7 +686,7 @@ task_main(int argc, char *argv[]) g_func_counts[i] = 0; } - g_task_should_exit = false; + g_task_should_exit.store(false); uORB::Publication dataman_response_pub{ORB_ID(dataman_response)}; const int dataman_request_sub = orb_subscribe(ORB_ID(dataman_request)); @@ -698,7 +701,7 @@ task_main(int argc, char *argv[]) int ret = g_dm_ops->initialize(max_offset); if (ret) { - g_task_should_exit = true; + g_task_should_exit.store(true); goto end; } @@ -825,7 +828,7 @@ task_main(int argc, char *argv[]) } /* time to go???? */ - if (g_task_should_exit) { + if (g_task_should_exit.load()) { break; } } @@ -856,9 +859,11 @@ start() px4_sem_setprotocol(&g_init_sema, SEM_PRIO_NONE); /* start the worker thread with low priority for disk IO */ - if (px4_task_spawn_cmd("dataman", SCHED_DEFAULT, SCHED_PRIORITY_DEFAULT - 10, - PX4_STACK_ADJUSTED(TASK_STACK_SIZE), task_main, - nullptr) < 0) { + g_task_id = px4_task_spawn_cmd("dataman", SCHED_DEFAULT, SCHED_PRIORITY_DEFAULT - 10, + PX4_STACK_ADJUSTED(TASK_STACK_SIZE), task_main, + nullptr); + + if (g_task_id < 0) { px4_sem_destroy(&g_init_sema); PX4_ERR("task start failed"); return -1; @@ -887,7 +892,7 @@ static void stop() { /* Tell the worker task to shut down */ - g_task_should_exit = true; + g_task_should_exit.store(true); } static void @@ -1017,6 +1022,8 @@ dataman_main(int argc, char *argv[]) if (!strcmp(argv[1], "stop")) { stop(); + px4_task_join(g_task_id); + g_task_id = -1; #ifdef CONFIG_DATAMAN_PERSISTENT_STORAGE free(k_data_manager_device_path); k_data_manager_device_path = nullptr;