feat(dataman): handle zero-size cache and clear pending responses on new request by JonasPerolini · Pull Request #27552 · PX4/PX4-Autopilot

JonasPerolini · 2026-06-02T17:43:11Z

Initially implemented for #26973 and split the large PR into smaller ones.

Changes:

Handle cache with zero size:
- Motivation: the geofence (_dataman_cache{"geofence_dm_cache_miss", 0}) creates its cache with size 0, and the
  mission/RTL caches resize to a count that can be 0 (e.g. _dataman_cache_safepoint.resize(_stats.num_items);).
- Changes to handle a zero-cache:
  - no allocation when empty,
  - resize(0) frees the buffer and resets state
  - no zero division or out-of-bounds indexing after a resize.
Stale responses: before sending a request, clear any queued reply on the response topic, so a response from an aborted or timed-out operation can't be matched to the next request.

Tested with unit tests in test_dataman.cpp:

zero-size cache
resize cache to zero
reduce size of cache
the stale-response queue clear

…new request

JonasPerolini · 2026-06-02T17:46:48Z

-	if (!_items) {
-		return false;
-	}


With a zero-size cache (a cache made with 0, or resized to 0), _items can be nullptr. The old if (!_items) return false; made loadWait() give up right away for an empty cache, so it never reached the readSync fallback even when the caller passed a timeout. That meant read-through stopped working whenever caching was turned off.

Instead of the early return, an empty cache now skips the cache lookup (there is nothing stored) but still continues to the readSync fallback, so callers with a zero-size cache still get their data from a direct read. The step that stores the result is also guarded by _items, so we never write into a null array.

github-actions · 2026-06-02T17:51:53Z

🔎 FLASH Analysis

px4_fmu-v5x [Total VM Diff: 192 byte (0.01 %)]

    FILE SIZE        VM SIZE    
 --------------  -------------- 
  +0.0%    +192  +0.0%    +192    .text
     +59%     +80   +59%     +80    DatamanCache::resize()
    [NEW]     +76  [NEW]     +76    DatamanClient::clearPendingResponse()
     +31%      +8   +31%      +8    DatamanCache::changeUpdateIndex()
    +3.1%      +8  +3.1%      +8    DatamanCache::loadWait()
    +3.6%      +4  +3.6%      +4    DatamanCache::DatamanCache()
     +12%      +4   +12%      +4    DatamanCache::~DatamanCache()
    +2.9%      +4  +2.9%      +4    DatamanClient::readAsync()
    +1.4%      +4  +1.4%      +4    DatamanClient::syncHandler()
    +1.7%      +4  +1.7%      +4    FeasibilityChecker::checkTakeoffLandAvailable()
  +0.0%    +155  [ = ]       0    .debug_abbrev
  +0.0%     +16  [ = ]       0    .debug_aranges
  +0.0%     +56  [ = ]       0    .debug_frame
  +0.0% +1.06Ki  [ = ]       0    .debug_info
  +0.0%    +457  [ = ]       0    .debug_line
   -80.0%      -4  [ = ]       0    [Unmapped]
    +0.0%    +461  [ = ]       0    [section .debug_line]
  +0.0%    +246  [ = ]       0    .debug_loclists
  +0.0%     +81  [ = ]       0    .debug_rnglists
    +100%      +1  [ = ]       0    [Unmapped]
    +0.0%     +80  [ = ]       0    [section .debug_rnglists]
  +0.0%    +117  [ = ]       0    .debug_str
  +0.4%      +1  [ = ]       0    .shstrtab
  +0.0%     +43  [ = ]       0    .strtab
    [NEW]     +43  [ = ]       0    DatamanClient::clearPendingResponse()
     +15%     +16  [ = ]       0    ___ZN39ControlAllocationSequentialDesaturation23computeDesaturationGainERKN6matrix6VectorIfLj16EEES4__veneer
   -42.1%     -16  [ = ]       0    __sched_unlock_veneer
  +0.0%     +48  [ = ]       0    .symtab
    [NEW]     +48  [ = ]       0    DatamanClient::clearPendingResponse()
     +67%     +32  [ = ]       0    ___ZN39ControlAllocationSequentialDesaturation23computeDesaturationGainERKN6matrix6VectorIfLj16EEES4__veneer
   -40.0%     -32  [ = ]       0    __sched_unlock_veneer
  -2.2%    -192  [ = ]       0    [Unmapped]
  +0.0% +2.25Ki  +0.0%    +192    TOTAL

px4_fmu-v6x [Total VM Diff: 192 byte (0.01 %)]

    FILE SIZE        VM SIZE    
 --------------  -------------- 
  +0.0%    +192  +0.0%    +192    .text
     +59%     +80   +59%     +80    DatamanCache::resize()
    [NEW]     +76  [NEW]     +76    DatamanClient::clearPendingResponse()
     +31%      +8   +31%      +8    DatamanCache::changeUpdateIndex()
    +3.1%      +8  +3.1%      +8    DatamanCache::loadWait()
    +3.6%      +4  +3.6%      +4    DatamanCache::DatamanCache()
     +12%      +4   +12%      +4    DatamanCache::~DatamanCache()
    +2.9%      +4  +2.9%      +4    DatamanClient::readAsync()
    +1.4%      +4  +1.4%      +4    DatamanClient::syncHandler()
    +1.7%      +4  +1.7%      +4    FeasibilityChecker::checkTakeoffLandAvailable()
  +0.0%    +155  [ = ]       0    .debug_abbrev
  +0.0%     +16  [ = ]       0    .debug_aranges
  +0.0%     +56  [ = ]       0    .debug_frame
  +0.0% +1.06Ki  [ = ]       0    .debug_info
  +0.0%    +465  [ = ]       0    .debug_line
    +133%      +4  [ = ]       0    [Unmapped]
    +0.0%    +461  [ = ]       0    [section .debug_line]
  +0.0%    +246  [ = ]       0    .debug_loclists
  +0.0%     +81  [ = ]       0    .debug_rnglists
     +50%      +1  [ = ]       0    [Unmapped]
    +0.0%     +80  [ = ]       0    [section .debug_rnglists]
  +0.0%    +117  [ = ]       0    .debug_str
  +0.4%      +1  [ = ]       0    .shstrtab
  +0.0%     +43  [ = ]       0    .strtab
    [NEW]     +43  [ = ]       0    DatamanClient::clearPendingResponse()
  +0.0%     +48  [ = ]       0    .symtab
    [NEW]     +48  [ = ]       0    DatamanClient::clearPendingResponse()
  -4.4%    -192  [ = ]       0    [Unmapped]
  +0.0% +2.26Ki  +0.0%    +192    TOTAL

Updated: 2026-06-15T19:50:08

JonasPerolini · 2026-06-14T14:59:25Z

Good evening @bkueng, could you please have a look at this PR?

bkueng

Nice work, this looks correct.
Sorry my response time is quite slow these days.

bkueng · 2026-06-15T12:00:52Z

+	if (!_dataman_client1.readAsync(item, index, scratch, length)) {
+		PX4_ERR("first readAsync failed");
+		return false;
+	}
+
+	// Give the dataman task time to answer while we deliberately skip update().
+	px4_usleep(100_ms);
+
+	// Abandon the operation. The queued reply is now stale.
+	_dataman_client1.abortCurrentOperation();
+
+	// A fresh async read must drain the stale reply and return the current value.
+	memset(_buffer_read, 0, sizeof(_buffer_read));
+
+	if (!_dataman_client1.readAsync(item, index, _buffer_read, length)) {
+		PX4_ERR("second readAsync failed");
+		return false;
+	}


Shouldn't these 2 reads operate on different indexes with different stored data, to ensure the right data is read?

Hey @bkueng, you're correct, if we re-read the same idx with the same data we don't know if clearPendingResponse worked as expected. Test updated here 01b93e2 to have different values and indexes.

Note that the new version still does not prove that clearPendingResponse works as expected. If clearPendingResponse breaks and the stale data is still there, once DatamanClient::update() is called for the new idx, this condition is not true: (response.index == _active_request.index) and therefore the stale response is skipped. On the next iteration, the new response will be used and the test will pass. (Confirmed by commenting out clearPendingResponse(); in readAsync, test still passes)

The proper test would be:

Write stale_value to index 7.

readAsync index 7 (queues the stale reply).

Abort.

Change index 7 data to new_value.

readAsync index 7, if it fails to clear the queue, it reads the stale_value

The problem is that I don't know how to perform step 4. I thought about using _dataman_client2 to change the backend but this would queue to the same uorb topic.

I'm wondering if the best option is to just remove this specific test. We could also keep it as it tests that stale responses are discarded when the response item does not match the active request (response.index == _active_request.index)

I've confirmed that clearPendingResponse works as expected by removing the check (response.index == _active_request.index) inside of update()

Test passes with clearPendingResponse inside of readAsync

Test fails when clearPendingResponse is commented out:

ERROR [tests] reissued read returned wrong data at 0 ERROR [tests] TEST FAILED: testAsyncAbortAndReissue

I think the test makes sense either way. It should not matter which internal mechanism prevents a stale response, as long as the overall result is as expected.

feat(dataman): handle zero-size cache and clear pending responses on …

5333e50

…new request

JonasPerolini requested a review from Igor-Misic June 2, 2026 17:43

github-actions Bot added kind:feature Request or change that adds new functionality. kind:test Adds or improves tests. scope:testing Unit, integration, fuzzing, or test data. labels Jun 2, 2026

JonasPerolini commented Jun 2, 2026

View reviewed changes

JonasPerolini requested a review from bkueng June 10, 2026 09:06

bkueng previously approved these changes Jun 15, 2026

View reviewed changes

fix(test): testAsyncAbortAndReissue ensure stale reply is cleared

01b93e2

JonasPerolini dismissed bkueng’s stale review via 01b93e2 June 15, 2026 19:43

bkueng approved these changes Jun 16, 2026

View reviewed changes

bkueng merged commit 13e74de into PX4:main Jun 16, 2026
73 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat(dataman): handle zero-size cache and clear pending responses on new request#27552

feat(dataman): handle zero-size cache and clear pending responses on new request#27552
bkueng merged 2 commits into
PX4:mainfrom
JonasPerolini:pr-dataman-zero-size

JonasPerolini commented Jun 2, 2026

Uh oh!

JonasPerolini Jun 2, 2026

Uh oh!

github-actions Bot commented Jun 2, 2026 •

edited

Loading

Uh oh!

JonasPerolini commented Jun 14, 2026

Uh oh!

bkueng left a comment

Uh oh!

bkueng Jun 15, 2026

Uh oh!

JonasPerolini Jun 15, 2026 •

edited

Loading

Uh oh!

JonasPerolini Jun 15, 2026 •

edited

Loading

Uh oh!

JonasPerolini Jun 16, 2026

Uh oh!

bkueng Jun 16, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

JonasPerolini commented Jun 2, 2026

Uh oh!

JonasPerolini Jun 2, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Jun 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔎 FLASH Analysis

Uh oh!

JonasPerolini commented Jun 14, 2026

Uh oh!

bkueng left a comment

Choose a reason for hiding this comment

Uh oh!

bkueng Jun 15, 2026

Choose a reason for hiding this comment

Uh oh!

JonasPerolini Jun 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JonasPerolini Jun 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JonasPerolini Jun 16, 2026

Choose a reason for hiding this comment

Uh oh!

bkueng Jun 16, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

github-actions Bot commented Jun 2, 2026 •

edited

Loading

JonasPerolini Jun 15, 2026 •

edited

Loading

JonasPerolini Jun 15, 2026 •

edited

Loading