Skip to content

Commit

Permalink
Fix exchange manager tests to handle losing timeslice better. (#27872)
Browse files Browse the repository at this point in the history
We could get into a situation where we lost the timeslice after the SendMessage
call and before we asserted the response timeout had not happened yet, which
would cause the test to fail.

The changes here are:

1) Move the assert that we are not timed out to _before_ SendMessage().  This
   ensures that our state is correct up front, and generally nothing under
   SendMessage proper or sending the message should trigger a timeout per se.
2) Use more slack when waiting for the timeout, just in case.

Fixes #27479
  • Loading branch information
bzbarsky-apple authored and pull[bot] committed Sep 21, 2023
1 parent 992b1e9 commit 2687403
Showing 1 changed file with 8 additions and 5 deletions.
13 changes: 8 additions & 5 deletions src/messaging/tests/TestExchangeMgr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -179,16 +179,19 @@ void CheckSessionExpirationDuringTimeout(nlTestSuite * inSuite, void * inContext
ExpireSessionFromTimeoutDelegate sendDelegate;
ExchangeContext * ec1 = ctx.NewExchangeToBob(&sendDelegate);

ec1->SetResponseTimeout(System::Clock::Timeout(100));
auto timeout = System::Clock::Timeout(100);
ec1->SetResponseTimeout(timeout);

NL_TEST_ASSERT(inSuite, !sendDelegate.IsOnResponseTimeoutCalled);

ec1->SendMessage(Protocols::BDX::Id, kMsgType_TEST1, System::PacketBufferHandle::New(System::PacketBuffer::kMaxSize),
SendFlags(Messaging::SendMessageFlags::kExpectResponse).Set(Messaging::SendMessageFlags::kNoAutoRequestAck));

ctx.DrainAndServiceIO();
NL_TEST_ASSERT(inSuite, !sendDelegate.IsOnResponseTimeoutCalled);

// Wait for our timeout to elapse. Give it an extra 100ms.
ctx.GetIOContext().DriveIOUntil(200_ms32, [&sendDelegate] { return sendDelegate.IsOnResponseTimeoutCalled; });
// Wait for our timeout to elapse. Give it an extra 1000ms of slack,
// because if we lose the timeslice for longer than the slack we could end
// up breaking out of the loop before the timeout timer has actually fired.
ctx.GetIOContext().DriveIOUntil(timeout + 1000_ms32, [&sendDelegate] { return sendDelegate.IsOnResponseTimeoutCalled; });

NL_TEST_ASSERT(inSuite, sendDelegate.IsOnResponseTimeoutCalled);

Expand Down

0 comments on commit 2687403

Please sign in to comment.