Os/Generic/LocklessPriorityQueue: TSan stress tests, adversarial scenarios, and CI workflow by devin-ai-integration[bot] · Pull Request #151 · JPL-Devin/fprime

devin-ai-integration · 2026-06-03T01:46:44Z


*Related Issue(s)*	nasa#5076
*Has Unit Tests (y/n)*	y
*Documentation Included (y/n)*	n
*Generative AI was used in this contribution (y/n)*	y

Change Description

Adds a ThreadSanitizer CI workflow and two test targets for the lockless priority queue:

New test file (LocklessPriorityQueueTsanTests.cpp):

TSan stress: 8P/8C × 40k msgs through 32-slot queue (iterated 20×), tiny-queue (depth=4) high contention, asymmetric producer/consumer ratios
Adversarial: depth-1 MPMC, full-queue storm (pre-fill then unleash producers), single-slot 1P/1C ping-pong (50k msgs, FIFO-order verified), high-water-mark accuracy, create/teardown/recreate cycles, sequence-wrap modular-comparison logic, mixed-priority contention, rapid drain-then-teardown

New CMake target: LocklessPriorityQueueTsanTest registered in Os/Generic/CMakeLists.txt.

New workflow (.github/workflows/tsan-lockless-queue.yml):

Generates with -fsanitize=thread -g -O1
Builds only LocklessPriorityQueueTest + LocklessPriorityQueueTsanTest (targeted cmake build)
Runs both under TSan with halt_on_error=1
Existing LocklessPriorityQueueTest (CommonTests + queue rules) also runs under TSan — reuses queue rules as-is

Rationale

The lockless queue uses lock-free atomics with release/acquire ordering. Data races and ABA bugs may only manifest under specific thread interleavings that normal tests rarely hit. TSan instruments every memory access and validates the happens-before graph, catching races even when the "lucky" scheduling produced a correct result.

Testing/Review Recommendations

Verify the TSan CI workflow triggers and both test targets pass green
Review adversarial scenarios for coverage of edge cases in the slot state machine
SequenceWrapComparison tests the modular unsigned subtraction directly — verify it matches isCandidatePreferred() in production

Future Work

ARM cross-architecture testing (Pi self-hosted runner or QEMU aarch64) for weak memory ordering validation
Nightly long-soak chaos test (10M+ random ops)
Relacy model checking for exhaustive small-config interleaving exploration

AI Usage (see policy)

Code generation, test design, and CI workflow authoring.

Link to Devin session: https://nasa-jpl-demo.devinenterprise.com/sessions/4fdd5501608d46b78b9a22f404376854

Adds a lockless implementation of Os::QueueInterface that targets flight-software constraints: all memory is allocated exactly once at create() time, every non-blocking control path is bounded by queue depth, and the non-blocking variants of send/receive use only lock-free atomics so that they can be invoked from interrupt context. The queue is a fixed pool of slots, each guarded by a packed (state, tag) atomic that drives a four-state lifecycle (FREE -> WRITING -> READY -> READING -> FREE). Producers and consumers coordinate through per-slot compare-exchange operations; consumers select the slot with the highest priority and break ties by sequence number, preserving the priority-and-FIFO ordering of Os::Generic::PriorityQueue. The new module is registered as Os_Generic_LocklessPriorityQueue alongside the existing Os_Generic_PriorityQueue, and a parallel test target LocklessPriorityQueueTest reuses the shared queue tests (Os/test/ut/queue/CommonTests.cpp and QueueRules.cpp) the same way the existing PriorityQueueTest does. An additional test file adds two lockless-specific scenarios that exercise the implementation with real OS threads. A new SDD section in docs/sdd.md documents the algorithm, memory ordering, bounded-loop budgets, and ISR-safety guarantees.

…efault Os_Queue Switches the default Os_Queue implementation chosen by the unix Platform config from Os_Generic_PriorityQueue to the new ISR-safe Os_Generic_LocklessPriorityQueue. Existing deployments (Ref, etc.) link against the lockless implementation through the same delegate mechanism.

The non-blocking paths remain strictly atomic-only. On BLOCKING send/receive, a 100 microsecond sleep_for is inserted between bounded scans when no slot can be claimed, so multiple active components that block on an empty queue do not collectively saturate the host CPU. The back-off is a scheduling hint, not a synchronization primitive, and is unreachable from the NONBLOCKING path so ISR callers are unaffected. SDD section 10 documents the rationale.

…teardown() Resource cleanup is now exclusively the responsibility of teardown(), matching the existing Os::Generic::PriorityQueue contract. The destructor is empty. Background: upstream CI (nasa#5076) showed FppTest_topology_special_ports and FppTest_topology_main aborting with 'pure virtual method called' at process exit. The cause was the destructor calling teardownInternal(), which calls Fw::MemAllocatorRegistry::getInstance() and dispatches the virtual MemAllocator::deallocate. The registry is itself a function-local static and can be destroyed before the queue's owning topology global, in which case the virtual call lands on a torn-down v-table. Fix: ~LocklessPriorityQueue() is now empty, exactly like Os::Generic::PriorityQueue::~PriorityQueue(). Owners must call teardown() explicitly before the queue (or its hosting Os::Queue) is destroyed. Tests already do this: Tester::~Tester() calls queue.teardown(), and every failure path in CommonTests.cpp invokes queue.teardown(). Three new lockless-specific tests cover the lifecycle contract: - LocklessLifetime.DestructWithoutCreate: destroying a never-created queue is safe (the destructor must perform no work). - LocklessLifetime.CreateTeardownDestruct: destroying a queue after explicit teardown is safe (no double-free). - LocklessLifetime.TeardownIsIdempotent: teardown() can be called repeatedly. Verified locally: 25/25 lockless tests pass under ASan/UBSan/LSan; 25/25 PriorityQueueTest still pass; FppTest 126/126 tests pass including #125 and Co-Authored-By: michael.d.starch <michael.d.starch@jpl.nasa.gov> #126 which previously aborted at exit.

…n test Three minor CI fixes for upstream PR nasa#5076 on top of the empty- destructor commit: - clang-format: re-flow lines that exceeded the project's 120-column limit in LocklessPriorityQueue.cpp and LocklessPriorityQueueTests.cpp. Output of 'fprime-util format --check --dirs Os' is now clean. - check-spelling: add 'acq', 'tiebreak', and 'LOCKLESSPRIORITYQUEUE' to the per-repo allow list .github/actions/spelling/expect.txt. 'acq' is the fragment of std::memory_order_acq_rel that the splitter sees on '_'; 'tiebreak' is used in comments describing the priority/sequence ordering; 'LOCKLESSPRIORITYQUEUE' is the include guard. - test_recursion: update cmake/test/data/cmake/target/test_recursion.cmake expected-deps list to reflect the new default Os_Queue implementation. Replace Os_Generic_PriorityQueue / Os_Generic_PriorityQueue_Implementation / Os_Generic_Types with Os_Generic_LocklessPriorityQueue and Os_Generic_LocklessPriorityQueue_Implementation. The lockless queue does not depend on Os_Generic_Types because it does not use MaxHeap. Verified locally: - 'fprime-util format --check --dirs Os' produces no errors. - 'pytest cmake/test/src/test_feature.py -k feature_run|framework|targets' passes. - LocklessPriorityQueueTest still 25/25 pass under ASan/UBSan/LSan. Co-Authored-By: michael.d.starch <michael.d.starch@jpl.nasa.gov>

- CPP-3: LOCKLESS_BLOCKING_BACKOFF_US int -> U32 (fixed-size type) - CPP-18: explicit QueueHandle() base constructor in member-initializer list - Test quality: replace tautological SUCCEED() in DestructWithoutCreate with observable assertion (getMessagesAvailable() == 0) so the test has a non-vacuous check even without sanitizers - CPP-25: replace std::vector with fixed-size arrays in concurrency tests; thread and atomic counts are compile-time constants - Documentation: list LocklessPriorityQueue in Os/Generic/docs/sdd.md (package-level SDD) and Os/docs/sdd.md section 4 (OSAL generic services) Co-Authored-By: michael.d.starch <michael.d.starch@jpl.nasa.gov>

Co-Authored-By: michael.d.starch <michael.d.starch@jpl.nasa.gov>

…scenarios, and CI workflow - Add LocklessPriorityQueueTsanTests.cpp with: - TSan stress: 8P/8C high-thread MPMC, tiny-queue contention, asymmetric producer/consumer ratios, iterated runs - Adversarial: depth-1 ping-pong, full-queue storm, single-slot ping-pong, high-water-mark accuracy, create/teardown/recreate cycles, sequence-wrap logic, mixed-priority contention, rapid drain-then-teardown - Register LocklessPriorityQueueTsanTest CMake target - Add .github/workflows/tsan-lockless-queue.yml: builds with -fsanitize=thread and runs both the existing lockless queue tests and the new stress/adversarial tests under ThreadSanitizer Co-Authored-By: michael.d.starch <michael.d.starch@jpl.nasa.gov>

devin-ai-integration · 2026-06-03T01:46:45Z

Original prompt from michael.d.starch

We have this PR fprime#5076 that introduces a locklesspriorityqueue. First, can you review it with @playbook:playbook-13e60200380742339b99dfd4b47d1fd3. Then can you fix any issues it raises. Make sure to follow the flight and c++ standards.

When done, please recommend a way to test / build confidence in it. Given that it could possibly raise very subtle concurrency bugs, we need an absolutely robust test. When you have a plan, let me know

devin-ai-integration · 2026-06-03T01:46:46Z

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

Address comments on this PR that start with 'DevinAI' or '@devin'.
Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

Disable automatic comment, CI, and merge conflict monitoring

Co-Authored-By: michael.d.starch <michael.d.starch@jpl.nasa.gov>

Both isOlder(0, topBit) and isOlder(topBit, 0) return true because the unsigned difference is 0x80000000 in both directions. The test incorrectly expected false for one direction. Fixed to EXPECT_TRUE for both, with added near-boundary tests where the comparison is well-defined. Co-Authored-By: michael.d.starch <michael.d.starch@jpl.nasa.gov>

All four near-boundary assertions had inverted expectations. E.g. isOlder(0, topBit+1) diff=0x7FFFFFFF MSB=0 => false, not true. Co-Authored-By: michael.d.starch <michael.d.starch@jpl.nasa.gov>

LeStarch · 2026-06-03T01:56:59Z

+          fetch-depth: 0
+          submodules: true
+
+      - uses: ./.github/actions/setup


Use the nasa/fprime-actions action

LeStarch · 2026-06-03T01:58:22Z

+
+      - name: "Generate UT build with TSan flags"
+        run: |
+          fprime-util generate --ut \


Fprime util has A tsan flag….use it

LeStarch · 2026-06-03T01:59:14Z

+
+      - uses: ./.github/actions/setup
+
+      - name: "Generate UT build with TSan flags"


Add to fprime actions for reusability

LeStarch · 2026-06-03T02:00:50Z

+            --target LocklessPriorityQueueTsanTest \
+            -j4
+
+      - name: "Run existing lockless queue tests under TSan"


Build and run with check

LeStarch and others added 8 commits May 1, 2026 03:01

spelling: add 'transitioning' to expect.txt

5772eef

Co-Authored-By: michael.d.starch <michael.d.starch@jpl.nasa.gov>

Fix spelling check: rename MPMC/hwm identifiers to full words

64ba01c

Co-Authored-By: michael.d.starch <michael.d.starch@jpl.nasa.gov>

This comment was marked as resolved.

Sign in to view

Fix SequenceWrapComparison: correct inverted near-boundary assertions

1155d52

All four near-boundary assertions had inverted expectations. E.g. isOlder(0, topBit+1) diff=0x7FFFFFFF MSB=0 => false, not true. Co-Authored-By: michael.d.starch <michael.d.starch@jpl.nasa.gov>

LeStarch reviewed Jun 3, 2026

View reviewed changes

devin-ai-integration Bot force-pushed the devin/1777603597-os-lockless-priority-queue branch from 5772eef to 2ea407d Compare June 3, 2026 02:07

devin-ai-integration Bot force-pushed the devin/1777603597-os-lockless-priority-queue branch from 0f7c38a to bfaf23a Compare August 4, 2026 22:41

Navigation Menu

Uh oh!

Os/Generic/LocklessPriorityQueue: TSan stress tests, adversarial scenarios, and CI workflow - #151

Os/Generic/LocklessPriorityQueue: TSan stress tests, adversarial scenarios, and CI workflow#151
devin-ai-integration[bot] wants to merge 11 commits into
devin/1777603597-os-lockless-priority-queuefrom
devin/1780450861-tsan-stress-tests

devin-ai-integration Bot commented Jun 3, 2026 •

edited

Loading

Uh oh!

devin-ai-integration Bot commented Jun 3, 2026

Uh oh!

devin-ai-integration Bot commented Jun 3, 2026

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

LeStarch Jun 3, 2026

Uh oh!

LeStarch Jun 3, 2026

Uh oh!

LeStarch Jun 3, 2026

Uh oh!

LeStarch Jun 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant


		- uses: ./.github/actions/setup

		- name: "Generate UT build with TSan flags"

Uh oh!

Conversation

devin-ai-integration Bot commented Jun 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Change Description

Rationale

Testing/Review Recommendations

Future Work

AI Usage (see policy)

Uh oh!

devin-ai-integration Bot commented Jun 3, 2026

Uh oh!

devin-ai-integration Bot commented Jun 3, 2026

🤖 Devin AI Engineer

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

LeStarch Jun 3, 2026

Choose a reason for hiding this comment

Uh oh!

LeStarch Jun 3, 2026

Choose a reason for hiding this comment

Uh oh!

LeStarch Jun 3, 2026

Choose a reason for hiding this comment

Uh oh!

LeStarch Jun 3, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

devin-ai-integration Bot commented Jun 3, 2026 •

edited

Loading