fix(parameters): fix data races in DynamicSparseLayer

Single TSan/ASan-clean implementation that works on both POSIX and NuttX:

- size()/byteSize() take the AtomicTransaction lock so they don't race
  with store() updating _next_slot/_n_slots.

- _grow() snapshots _n_slots under the lock for sizing the malloc, so
  another thread's growth between the malloc and the memcpy can't make
  the memcpy exceed the allocation. Both platforms use the unlock-
  around-malloc + CAS-retry pattern: NuttX requires it (malloc can't
  be called with IRQs disabled), and on POSIX it's safe because the
  AtomicTransaction mutex still serializes everything else and the
  brief unlock window doesn't touch shared state.

- ~DynamicSparseLayer() takes the lock, zeros _next_slot / _n_slots,
  stores nullptr into _slots, then frees the buffer. This fixes the
  heap-use-after-free TSan caught at process exit on POSIX, where the
  static DynamicSparseLayer instances in parameters.cpp are torn down
  by cxa_atexit while other threads (commander etc.) are still calling
  param_get(). Any reader that acquires the lock post-destruction sees
  _next_slot == 0 and falls through to the parent layer; the parent
  chain in parameters.cpp is in reverse-declaration order, so the
  parent is still alive when the child is destroyed.

Add DynamicSparseLayerTest with concurrent stress tests that reproduce
the races. ConcurrentMultipleWriters pre-allocates to TOTAL to avoid
the known ABA limitation in the CAS retry loop with concurrent _grow().

Drive-by: drop the unused void *p member from param_value_u — nothing
in the tree reads or writes it, and on 64-bit POSIX it was silently
doubling the size of every parameter slot.

Originally found while running the new SIH-based CI tests.
This commit is contained in:
Julian Oes
2026-07-02 08:42:49 -07:00
committed by Ramon Roche
parent 692d624670
commit 02ecfd4779
4 changed files with 413 additions and 44 deletions
+1
View File
@@ -221,3 +221,4 @@ if(${PX4_PLATFORM} STREQUAL "posix" OR ${PX4_PLATFORM} STREQUAL "ros2")
endif()
px4_add_functional_gtest(SRC ParameterTest.cpp LINKLIBS parameters)
px4_add_functional_gtest(SRC DynamicSparseLayerTest.cpp LINKLIBS parameters)
File diff suppressed because it is too large Load Diff
File diff suppressed because it is too large Load Diff
-1
View File
@@ -475,7 +475,6 @@ __EXPORT void param_control_autosave(bool enable);
* Parameter value union.
*/
union param_value_u {
void *p;
int32_t i;
float f;
};