diff --git a/Svc/ComAggregator/ComAggregator.cpp b/Svc/ComAggregator/ComAggregator.cpp index c4edf6d8be7..2e313f99600 100644 --- a/Svc/ComAggregator/ComAggregator.cpp +++ b/Svc/ComAggregator/ComAggregator.cpp @@ -8,6 +8,12 @@ namespace Svc { +namespace { +//! Queue slots that must remain free for a timeout signal to be enqueued: the timeout itself plus one +//! in-flight 'fill' and one in-flight 'status' signal (each bounded to one message by the com protocol). +constexpr FwSizeType TIMEOUT_QUEUE_HEADROOM = 3; +} // namespace + // ---------------------------------------------------------------------- // Component construction and destruction // ---------------------------------------------------------------------- @@ -55,7 +61,16 @@ void ComAggregator ::timeout_handler(FwIndexType portNum, U32 context) { // // Behaviorally, this solution will work exactly like the naive implementation with an infinite queue depth, but // prevents queue overflow when using finite queues. - if (this->m_allow_timeout) { + // + // Even so, timeout remains the only signal source without flow control: 'fill' and 'status' are each bounded + // to one in-flight message by the com protocol, but the rate group keeps delivering ticks while this + // component's dispatch thread is stalled (downstream backpressure, thread starvation). In the FILL state + // (m_allow_timeout true) such a stall would still fill the queue with timeout signals and trip the queue-full + // assertion in the autocoded signal send. Ticks are periodic and idempotent, so additionally skip the signal + // unless the queue has headroom for it plus the in-flight flow-controlled signals; a skipped tick is simply + // retried on the next cycle. + if (this->m_allow_timeout && + (this->m_queue.getMessagesAvailable() + TIMEOUT_QUEUE_HEADROOM <= this->m_queue.getDepth())) { this->aggregationMachine_sendSignal_timeout(); } } diff --git a/Svc/ComAggregator/test/ut/ComAggregatorTestMain.cpp b/Svc/ComAggregator/test/ut/ComAggregatorTestMain.cpp index 5c118ae0087..44bd5914aca 100644 --- a/Svc/ComAggregator/test/ut/ComAggregatorTestMain.cpp +++ b/Svc/ComAggregator/test/ut/ComAggregatorTestMain.cpp @@ -63,6 +63,12 @@ TEST(OffNominal, TimeoutOverflowPrevention) { tester.test_timeout(); } +TEST(OffNominal, TimeoutOverflowPreventionFillState) { + Svc::ComAggregatorTester tester; + tester.test_initial(); + tester.test_timeout_overflow_prevention_fill_state(); +} + TEST(Nominal, HoldWhileWaiting) { Svc::ComAggregatorTester tester; tester.test_initial(); diff --git a/Svc/ComAggregator/test/ut/ComAggregatorTester.cpp b/Svc/ComAggregator/test/ut/ComAggregatorTester.cpp index 98c2055fab5..b41ee15dd82 100644 --- a/Svc/ComAggregator/test/ut/ComAggregatorTester.cpp +++ b/Svc/ComAggregator/test/ut/ComAggregatorTester.cpp @@ -266,6 +266,48 @@ void ComAggregatorTester ::test_timeout_overflow_prevention() { this->clearHistory(); } +//! Tests that a stalled dispatch thread in the FILL state cannot flood the queue with timeout +//! signals past its depth, and that the component recovers cleanly once draining resumes +void ComAggregatorTester ::test_timeout_overflow_prevention_fill_state() { + // Precondition: initial has run. The state machine is now in the FILL state + // (m_allow_timeout == true) with an empty aggregation buffer and an empty queue. Unlike + // test_timeout_overflow_prevention (which drives m_allow_timeout to false via a comStatusIn + // failure before flooding, exercising the WAIT_STATUS-adjacent gate from the pre-existing + // fix), this test floods while m_allow_timeout stays true, exercising the FILL-state gap. + ASSERT_EQ(this->component.m_queue.getMessagesAvailable(), 0); + + // Simulate a stalled dispatch thread: the rate group keeps delivering 'timeout' ticks (this + // port is invoked directly here, exactly as a rate-group member call would) while nothing + // drains the component's message queue via dispatchOne. Loop well past the queue depth -- + // without the FILL-state headroom check, this would keep enqueueing right up to depth and + // trip the autocoded queue-full assertion inside aggregationMachine_sendSignal_timeout. + for (FwSizeType i = 0; i < static_cast(TEST_INSTANCE_QUEUE_DEPTH) * 2; i++) { + this->invoke_to_timeout(0, 0); + // The queue must never be allowed to reach its full depth: headroom is reserved for the + // (at most one each) in-flight 'fill'/'status' signals, so FILL-state flooding cannot trip + // the queue-full assert no matter how long the simulated stall runs. + ASSERT_LT(this->component.m_queue.getMessagesAvailable(), static_cast(TEST_INSTANCE_QUEUE_DEPTH)); + } + // With the aggregation buffer empty, isNotEmpty is false, so every dispatched timeout is a + // no-op transition (see test_timeout_zero); flooding has no side effects of its own. + ASSERT_from_dataOut_SIZE(0); + + // Confirm recovery once the (simulated) dispatch stall clears and draining resumes: every + // queued signal is still processable without error, and the queue returns to empty. + // (dispatchOne's underlying doDispatch() issues a BLOCKING queue receive, so drain exactly the + // snapshotted number of queued messages via dispatchCurrentMessages rather than looping until + // an "empty" status that a blocking dispatch will never produce.) + ASSERT_EQ(this->dispatchCurrentMessages(this->component), + Svc::ComAggregatorComponentBase::MsgDispatchStatus::MSG_DISPATCH_OK); + ASSERT_EQ(this->component.m_queue.getMessagesAvailable(), 0); + ASSERT_from_dataOut_SIZE(0); + this->clearHistory(); + + // Normal fill/timeout operation resumes cleanly after the flood and drain. + this->test_fill_multi(); + this->test_timeout(); +} + void ComAggregatorTester ::test_timeout_zero() { // Precondition: initialize has run this->invoke_to_timeout(0, 0); diff --git a/Svc/ComAggregator/test/ut/ComAggregatorTester.hpp b/Svc/ComAggregator/test/ut/ComAggregatorTester.hpp index 1ea0a1dcc23..856a323f455 100644 --- a/Svc/ComAggregator/test/ut/ComAggregatorTester.hpp +++ b/Svc/ComAggregator/test/ut/ComAggregatorTester.hpp @@ -65,6 +65,10 @@ class ComAggregatorTester final : public ComAggregatorGTestBase { //! Tests timeout operation void test_timeout_overflow_prevention(); + //! Tests that a stalled dispatch thread in the FILL state cannot flood the queue with timeout + //! signals past its depth, and that the component recovers cleanly once draining resumes + void test_timeout_overflow_prevention_fill_state(); + //! Tests timeout operation sends no empty buffer void test_timeout_zero();