Skip to content

Commit

Permalink
Fix TestReliableMessageProtocol timing bug. (#27817)
Browse files Browse the repository at this point in the history
  • Loading branch information
@bzbarsky-apple @pull
bzbarsky-apple authored and pull[bot] committed Oct 10, 2023
1 parent a6cb0ee commit c6cad50
Showing 1 changed file with 56 additions and 40 deletions.
96 changes: 56 additions & 40 deletions src/messaging/tests/TestReliableMessageProtocol.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,8 @@ constexpr auto retryBoosterTimeout = CHIP_CONFIG_RMP_DEFAULT_MAX_RETRANS * CHIP_
class MockAppDelegate : public UnsolicitedMessageHandler, public ExchangeDelegate
{
public:
MockAppDelegate(TestContext & ctx) : mTestContext(ctx) {}

CHIP_ERROR OnUnsolicitedMessageReceived(const PayloadHeader & payloadHeader, ExchangeDelegate *& newDelegate) override
{
// Handle messages by myself
Expand All @@ -89,6 +91,9 @@ class MockAppDelegate : public UnsolicitedMessageHandler, public ExchangeDelegat
auto * rc = ec->GetReliableMessageContext();
if (rc->HasPiggybackAckPending())
{
// Make sure we don't accidentally retransmit and end up acking
// the retransmit.
rc->GetReliableMessageMgr()->StopTimer();
(void) rc->TakePendingPeerAckMessageCounter();
}
}
Expand Down Expand Up @@ -127,12 +132,27 @@ class MockAppDelegate : public UnsolicitedMessageHandler, public ExchangeDelegat
}
}

void SetDropAckResponse(bool dropResponse)
{
mDropAckResponse = dropResponse;
if (!mDropAckResponse)
{
// Restart the MRP retransmit timer, now that we are not going to be
// dropping acks anymore, so we send out pending retransmits, if
// any, as needed.
mTestContext.GetExchangeManager().GetReliableMessageMgr()->StartTimer();
}
}

bool IsOnMessageReceivedCalled = false;
bool mReceivedPiggybackAck = false;
bool mDropAckResponse = false;
bool mRetainExchange = false;
ExchangeContext * mExchange = nullptr;
nlTestSuite * mTestSuite = nullptr;

private:
TestContext & mTestContext;
bool mDropAckResponse = false;
};

class MockSessionEstablishmentExchangeDispatch : public Messaging::ApplicationExchangeDispatch
Expand Down Expand Up @@ -279,7 +299,7 @@ void CheckAddClearRetrans(nlTestSuite * inSuite, void * inContext)
{
TestContext & ctx = *reinterpret_cast<TestContext *>(inContext);

MockAppDelegate mockAppDelegate;
MockAppDelegate mockAppDelegate(ctx);
ExchangeContext * exchange = ctx.NewExchangeToAlice(&mockAppDelegate);
NL_TEST_ASSERT(inSuite, exchange != nullptr);

Expand Down Expand Up @@ -337,7 +357,7 @@ void CheckResendApplicationMessage(nlTestSuite * inSuite, void * inContext)

CHIP_ERROR err = CHIP_NO_ERROR;

MockAppDelegate mockSender;
MockAppDelegate mockSender(ctx);
// TODO: temporarily create a SessionHandle from node id, will be fix in PR 3602
ExchangeContext * exchange = ctx.NewExchangeToAlice(&mockSender);
NL_TEST_ASSERT(inSuite, exchange != nullptr);
Expand Down Expand Up @@ -450,7 +470,7 @@ void CheckCloseExchangeAndResendApplicationMessage(nlTestSuite * inSuite, void *

CHIP_ERROR err = CHIP_NO_ERROR;

MockAppDelegate mockSender;
MockAppDelegate mockSender(ctx);
// TODO: temporarily create a SessionHandle from node id, will be fixed in PR 3602
ExchangeContext * exchange = ctx.NewExchangeToAlice(&mockSender);
NL_TEST_ASSERT(inSuite, exchange != nullptr);
Expand Down Expand Up @@ -594,13 +614,13 @@ void CheckResendApplicationMessageWithPeerExchange(nlTestSuite * inSuite, void *

CHIP_ERROR err = CHIP_NO_ERROR;

MockAppDelegate mockReceiver;
MockAppDelegate mockReceiver(ctx);
err = ctx.GetExchangeManager().RegisterUnsolicitedMessageHandlerForType(Echo::MsgType::EchoRequest, &mockReceiver);
NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR);

mockReceiver.mTestSuite = inSuite;

MockAppDelegate mockSender;
MockAppDelegate mockSender(ctx);
ExchangeContext * exchange = ctx.NewExchangeToAlice(&mockSender);
NL_TEST_ASSERT(inSuite, exchange != nullptr);

Expand Down Expand Up @@ -656,13 +676,13 @@ void CheckDuplicateMessageClosedExchange(nlTestSuite * inSuite, void * inContext

CHIP_ERROR err = CHIP_NO_ERROR;

MockAppDelegate mockReceiver;
MockAppDelegate mockReceiver(ctx);
err = ctx.GetExchangeManager().RegisterUnsolicitedMessageHandlerForType(Echo::MsgType::EchoRequest, &mockReceiver);
NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR);

mockReceiver.mTestSuite = inSuite;

MockAppDelegate mockSender;
MockAppDelegate mockSender(ctx);
ExchangeContext * exchange = ctx.NewExchangeToAlice(&mockSender);
NL_TEST_ASSERT(inSuite, exchange != nullptr);

Expand All @@ -681,8 +701,8 @@ void CheckDuplicateMessageClosedExchange(nlTestSuite * inSuite, void * inContext
loopback.mDroppedMessageCount = 0;

// Drop the ack, and also close the peer exchange
mockReceiver.mDropAckResponse = true;
mockReceiver.mRetainExchange = false;
mockReceiver.SetDropAckResponse(true);
mockReceiver.mRetainExchange = false;

// Ensure the retransmit table is empty right now
NL_TEST_ASSERT(inSuite, rm->TestGetCountRetransTable() == 0);
Expand All @@ -698,7 +718,7 @@ void CheckDuplicateMessageClosedExchange(nlTestSuite * inSuite, void * inContext
NL_TEST_ASSERT(inSuite, rm->TestGetCountRetransTable() == 1);

// Let's not drop the duplicate message
mockReceiver.mDropAckResponse = false;
mockReceiver.SetDropAckResponse(false);

err = ctx.GetExchangeManager().UnregisterUnsolicitedMessageHandlerForType(Echo::MsgType::EchoRequest);
NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR);
Expand All @@ -723,13 +743,13 @@ void CheckDuplicateOldMessageClosedExchange(nlTestSuite * inSuite, void * inCont

CHIP_ERROR err = CHIP_NO_ERROR;

MockAppDelegate mockReceiver;
MockAppDelegate mockReceiver(ctx);
err = ctx.GetExchangeManager().RegisterUnsolicitedMessageHandlerForType(Echo::MsgType::EchoRequest, &mockReceiver);
NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR);

mockReceiver.mTestSuite = inSuite;

MockAppDelegate mockSender;
MockAppDelegate mockSender(ctx);
ExchangeContext * exchange = ctx.NewExchangeToAlice(&mockSender);
NL_TEST_ASSERT(inSuite, exchange != nullptr);

Expand All @@ -748,8 +768,8 @@ void CheckDuplicateOldMessageClosedExchange(nlTestSuite * inSuite, void * inCont
loopback.mDroppedMessageCount = 0;

// Drop the ack, and also close the peer exchange
mockReceiver.mDropAckResponse = true;
mockReceiver.mRetainExchange = false;
mockReceiver.SetDropAckResponse(true);
mockReceiver.mRetainExchange = false;

// Ensure the retransmit table is empty right now
NL_TEST_ASSERT(inSuite, rm->TestGetCountRetransTable() == 0);
Expand All @@ -764,10 +784,6 @@ void CheckDuplicateOldMessageClosedExchange(nlTestSuite * inSuite, void * inCont
NL_TEST_ASSERT(inSuite, loopback.mDroppedMessageCount == 0);
NL_TEST_ASSERT(inSuite, rm->TestGetCountRetransTable() == 1);

// Make sure we don't accidentally retransmit before we are done with our
// message counter incrementing.
rm->StopTimer();

// Now send CHIP_CONFIG_MESSAGE_COUNTER_WINDOW_SIZE + 2 messages to make
// sure our original message is out of the message counter window. These
// messages can be sent withour MRP, because we are not expecting acks for
Expand Down Expand Up @@ -798,7 +814,7 @@ void CheckDuplicateOldMessageClosedExchange(nlTestSuite * inSuite, void * inCont
}

// Let's not drop the duplicate message's ack.
mockReceiver.mDropAckResponse = false;
mockReceiver.SetDropAckResponse(false);

err = ctx.GetExchangeManager().UnregisterUnsolicitedMessageHandlerForType(Echo::MsgType::EchoRequest);
NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR);
Expand Down Expand Up @@ -892,13 +908,13 @@ void CheckDuplicateMessage(nlTestSuite * inSuite, void * inContext)

CHIP_ERROR err = CHIP_NO_ERROR;

MockAppDelegate mockReceiver;
MockAppDelegate mockReceiver(ctx);
err = ctx.GetExchangeManager().RegisterUnsolicitedMessageHandlerForType(Echo::MsgType::EchoRequest, &mockReceiver);
NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR);

mockReceiver.mTestSuite = inSuite;

MockAppDelegate mockSender;
MockAppDelegate mockSender(ctx);
ExchangeContext * exchange = ctx.NewExchangeToAlice(&mockSender);
NL_TEST_ASSERT(inSuite, exchange != nullptr);

Expand All @@ -917,8 +933,8 @@ void CheckDuplicateMessage(nlTestSuite * inSuite, void * inContext)
loopback.mDroppedMessageCount = 0;

// Drop the ack, and keep the exchange around to receive the duplicate message
mockReceiver.mDropAckResponse = true;
mockReceiver.mRetainExchange = true;
mockReceiver.SetDropAckResponse(true);
mockReceiver.mRetainExchange = true;

// Ensure the retransmit table is empty right now
NL_TEST_ASSERT(inSuite, rm->TestGetCountRetransTable() == 0);
Expand All @@ -937,8 +953,8 @@ void CheckDuplicateMessage(nlTestSuite * inSuite, void * inContext)
NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR);

// Let's not drop the duplicate message
mockReceiver.mDropAckResponse = false;
mockReceiver.mRetainExchange = false;
mockReceiver.SetDropAckResponse(false);
mockReceiver.mRetainExchange = false;

// Wait for the first re-transmit and ack (should take 64ms)
ctx.GetIOContext().DriveIOUntil(1000_ms32, [&] { return loopback.mSentMessageCount >= 3; });
Expand All @@ -962,13 +978,13 @@ void CheckReceiveAfterStandaloneAck(nlTestSuite * inSuite, void * inContext)

CHIP_ERROR err = CHIP_NO_ERROR;

MockAppDelegate mockReceiver;
MockAppDelegate mockReceiver(ctx);
err = ctx.GetExchangeManager().RegisterUnsolicitedMessageHandlerForType(Echo::MsgType::EchoRequest, &mockReceiver);
NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR);

mockReceiver.mTestSuite = inSuite;

MockAppDelegate mockSender;
MockAppDelegate mockSender(ctx);
ExchangeContext * exchange = ctx.NewExchangeToAlice(&mockSender);
NL_TEST_ASSERT(inSuite, exchange != nullptr);

Expand Down Expand Up @@ -1052,13 +1068,13 @@ void CheckPiggybackAfterPiggyback(nlTestSuite * inSuite, void * inContext)

CHIP_ERROR err = CHIP_NO_ERROR;

MockAppDelegate mockReceiver;
MockAppDelegate mockReceiver(ctx);
err = ctx.GetExchangeManager().RegisterUnsolicitedMessageHandlerForType(Echo::MsgType::EchoRequest, &mockReceiver);
NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR);

mockReceiver.mTestSuite = inSuite;

MockAppDelegate mockSender;
MockAppDelegate mockSender(ctx);
ExchangeContext * exchange = ctx.NewExchangeToAlice(&mockSender);
NL_TEST_ASSERT(inSuite, exchange != nullptr);

Expand Down Expand Up @@ -1191,7 +1207,7 @@ void CheckSendUnsolicitedStandaloneAckMessage(nlTestSuite * inSuite, void * inCo

CHIP_ERROR err = CHIP_NO_ERROR;

MockAppDelegate mockSender;
MockAppDelegate mockSender(ctx);
ExchangeContext * exchange = ctx.NewExchangeToAlice(&mockSender);
NL_TEST_ASSERT(inSuite, exchange != nullptr);

Expand Down Expand Up @@ -1229,7 +1245,7 @@ void CheckSendStandaloneAckMessage(nlTestSuite * inSuite, void * inContext)
{
TestContext & ctx = *reinterpret_cast<TestContext *>(inContext);

MockAppDelegate mockAppDelegate;
MockAppDelegate mockAppDelegate(ctx);
ExchangeContext * exchange = ctx.NewExchangeToAlice(&mockAppDelegate);
NL_TEST_ASSERT(inSuite, exchange != nullptr);

Expand Down Expand Up @@ -1268,13 +1284,13 @@ void CheckMessageAfterClosed(nlTestSuite * inSuite, void * inContext)

CHIP_ERROR err = CHIP_NO_ERROR;

MockAppDelegate mockReceiver;
MockAppDelegate mockReceiver(ctx);
err = ctx.GetExchangeManager().RegisterUnsolicitedMessageHandlerForType(Echo::MsgType::EchoRequest, &mockReceiver);
NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR);

mockReceiver.mTestSuite = inSuite;

MockAppDelegate mockSender;
MockAppDelegate mockSender(ctx);
ExchangeContext * exchange = ctx.NewExchangeToAlice(&mockSender);
NL_TEST_ASSERT(inSuite, exchange != nullptr);

Expand Down Expand Up @@ -1388,13 +1404,13 @@ void CheckLostResponseWithPiggyback(nlTestSuite * inSuite, void * inContext)

CHIP_ERROR err = CHIP_NO_ERROR;

MockAppDelegate mockReceiver;
MockAppDelegate mockReceiver(ctx);
err = ctx.GetExchangeManager().RegisterUnsolicitedMessageHandlerForType(Echo::MsgType::EchoRequest, &mockReceiver);
NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR);

mockReceiver.mTestSuite = inSuite;

MockAppDelegate mockSender;
MockAppDelegate mockSender(ctx);
ExchangeContext * exchange = ctx.NewExchangeToAlice(&mockSender);
NL_TEST_ASSERT(inSuite, exchange != nullptr);

Expand Down Expand Up @@ -1543,13 +1559,13 @@ void CheckLostStandaloneAck(nlTestSuite * inSuite, void * inContext)

CHIP_ERROR err = CHIP_NO_ERROR;

MockAppDelegate mockReceiver;
MockAppDelegate mockReceiver(ctx);
err = ctx.GetExchangeManager().RegisterUnsolicitedMessageHandlerForType(Echo::MsgType::EchoRequest, &mockReceiver);
NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR);

mockReceiver.mTestSuite = inSuite;

MockAppDelegate mockSender;
MockAppDelegate mockSender(ctx);
ExchangeContext * exchange = ctx.NewExchangeToAlice(&mockSender);
NL_TEST_ASSERT(inSuite, exchange != nullptr);

Expand All @@ -1574,7 +1590,7 @@ void CheckLostStandaloneAck(nlTestSuite * inSuite, void * inContext)
mockSender.mRetainExchange = true;

// And ensure the ack heading back our way is dropped.
mockReceiver.mDropAckResponse = true;
mockReceiver.SetDropAckResponse(true);

err = exchange->SendMessage(Echo::MsgType::EchoRequest, std::move(buffer), SendFlags(SendMessageFlags::kExpectResponse));
NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR);
Expand All @@ -1596,7 +1612,7 @@ void CheckLostStandaloneAck(nlTestSuite * inSuite, void * inContext)
NL_TEST_ASSERT(inSuite, !receiverRc->IsAckPending());

// Don't drop any more acks.
mockReceiver.mDropAckResponse = false;
mockReceiver.SetDropAckResponse(false);

// Now send a message from the other side.
buffer = chip::MessagePacketBuffer::NewWithData(PAYLOAD, sizeof(PAYLOAD));
Expand Down

0 comments on commit c6cad50

Please sign in to comment.