From a6e33eb44b34ce5b8a44a812d45a9da6f5301466 Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Mon, 20 Jun 2022 15:16:07 -0400 Subject: [PATCH] Fix crashes when closing all exchanges for fabric. Our "close all exchanges except this special one for this fabric" API messes up exchange refcounting, leading to use-after-free. The fix is to reuse, as much as possible, the normal "session is going away" flow to notify exchanges, and other session consumers, that the sessions are in fact going away. Fixes https://github.com/project-chip/connectedhomeip/issues/19747 --- src/messaging/ExchangeContext.cpp | 22 +- src/messaging/ExchangeContext.h | 26 ++ src/messaging/ExchangeMgr.cpp | 18 -- src/messaging/ExchangeMgr.h | 5 - src/messaging/ReliableMessageContext.h | 15 -- src/messaging/tests/BUILD.gn | 11 +- src/messaging/tests/MessagingContext.h | 2 - .../tests/TestAbortExchangesForFabric.cpp | 235 ++++++++++++++++++ src/messaging/tests/TestExchangeMgr.cpp | 2 - src/messaging/tests/TestExchangeMgrDriver.cpp | 35 --- src/messaging/tests/TestMessagingLayer.h | 36 --- .../tests/TestReliableMessageProtocol.cpp | 2 - .../TestReliableMessageProtocolDriver.cpp | 35 --- src/transport/SecureSession.cpp | 10 +- src/transport/SecureSession.h | 8 +- src/transport/SessionManager.cpp | 6 +- src/transport/SessionManager.h | 4 +- 17 files changed, 305 insertions(+), 167 deletions(-) create mode 100644 src/messaging/tests/TestAbortExchangesForFabric.cpp delete mode 100644 src/messaging/tests/TestExchangeMgrDriver.cpp delete mode 100644 src/messaging/tests/TestMessagingLayer.h delete mode 100644 src/messaging/tests/TestReliableMessageProtocolDriver.cpp diff --git a/src/messaging/ExchangeContext.cpp b/src/messaging/ExchangeContext.cpp index 422a2f88e34ce4..d76e75c85be44e 100644 --- a/src/messaging/ExchangeContext.cpp +++ b/src/messaging/ExchangeContext.cpp @@ -326,7 +326,13 @@ ExchangeContext::~ExchangeContext() VerifyOrDie(!IsAckPending()); if (ReleaseSessionOnDestruction() && mSession) - mSession->AsSecureSession()->MarkForRemoval(); + { + // Move the session out of the older for the removal, so it doesn't try + // to notify us in our destructor. + const auto & session = mSession.Get(); + mSession.Release(); + session.Value()->AsSecureSession()->MarkForRemoval(); + } #if CONFIG_DEVICE_LAYER && CHIP_DEVICE_CONFIG_ENABLE_SED // Make sure that the exchange withdraws the request for Sleepy End Device active mode. @@ -573,5 +579,19 @@ ExchangeMessageDispatch & ExchangeContext::GetMessageDispatch(bool isEphemeralEx return ApplicationExchangeDispatch::Instance(); } +void ExchangeContext::AbortAllOtherCommunicationOnFabric() +{ + if (!mSession || !mSession->IsSecureSession()) + { + return; + } + + GetExchangeMgr()->GetSessionManager()->ReleaseSessionsForFabricExceptOne(mSession->GetFabricIndex(), mSession.Get().Value()); + + mSession->AsSecureSession()->MarkInactive(mSession); + + SetAutoReleaseSession(); +} + } // namespace Messaging } // namespace chip diff --git a/src/messaging/ExchangeContext.h b/src/messaging/ExchangeContext.h index b59c68cba17fb1..7131af44252438 100644 --- a/src/messaging/ExchangeContext.h +++ b/src/messaging/ExchangeContext.h @@ -184,6 +184,17 @@ class DLL_EXPORT ExchangeContext : public ReliableMessageContext, // using this function is recommended. void SetResponseTimeout(Timeout timeout); + // This API is used by commands that need to shut down all existing + // sessions/exchanges on a fabric but need to make sure the response to the + // command still goes out on the exchange the command came in on. This API + // will ensure that all secure sessions for the fabric this exchanges is on + // are released except the one this exchange is using, and will release + // that session once this exchange is done sending the response. + // + // This API is a no-op if called on an exchange that is not using a + // SecureSession. + void AbortAllOtherCommunicationOnFabric(); + private: Timeout mResponseTimeout{ 0 }; // Maximum time to wait for response (in milliseconds); 0 disables response timeout. ExchangeDelegate * mDelegate = nullptr; @@ -274,7 +285,22 @@ class DLL_EXPORT ExchangeContext : public ReliableMessageContext, void UpdateSEDIntervalMode(bool activeMode); static ExchangeMessageDispatch & GetMessageDispatch(bool isEphemeralExchange, ExchangeDelegate * delegate); + + // If SetAutoReleaseSession() is called, this exchange must be using a SecureSession, and should + // evict it when the exchange is done with all its work (including any MRP traffic). + inline void SetAutoReleaseSession(); + inline bool ReleaseSessionOnDestruction(); }; +inline void ExchangeContext::SetAutoReleaseSession() +{ + mFlags.Set(Flags::kFlagAutoReleaseSession, true); +} + +inline bool ExchangeContext::ReleaseSessionOnDestruction() +{ + return mFlags.Has(Flags::kFlagAutoReleaseSession); +} + } // namespace Messaging } // namespace chip diff --git a/src/messaging/ExchangeMgr.cpp b/src/messaging/ExchangeMgr.cpp index b01d5b4fe4cb5c..2c1242aff55c1c 100644 --- a/src/messaging/ExchangeMgr.cpp +++ b/src/messaging/ExchangeMgr.cpp @@ -376,23 +376,5 @@ void ExchangeManager::CloseAllContextsForDelegate(const ExchangeDelegate * deleg }); } -void ExchangeManager::AbortExchangesForFabricExceptOne(FabricIndex fabricIndex, ExchangeContext * deferred) -{ - VerifyOrDie(deferred->HasSessionHandle() && deferred->GetSessionHandle()->IsSecureSession()); - - mContextPool.ForEachActiveObject([&](auto * ec) { - if (ec->HasSessionHandle() && ec->GetSessionHandle()->GetFabricIndex() == fabricIndex) - { - if (ec == deferred) - ec->SetAutoReleaseSession(); - else - ec->Abort(); - } - return Loop::Continue; - }); - - mSessionManager->ReleaseSessionsForFabricExceptOne(fabricIndex, deferred->GetSessionHandle()); -} - } // namespace Messaging } // namespace chip diff --git a/src/messaging/ExchangeMgr.h b/src/messaging/ExchangeMgr.h index 112cddd82c7ecb..d6b775436799f4 100644 --- a/src/messaging/ExchangeMgr.h +++ b/src/messaging/ExchangeMgr.h @@ -183,11 +183,6 @@ class DLL_EXPORT ExchangeManager : public SessionMessageDelegate */ void CloseAllContextsForDelegate(const ExchangeDelegate * delegate); - // This API is used by commands that need to shut down all existing exchanges on a fabric but need to make sure the response to - // the command still goes out on the exchange the command came in on. This API flags that one exchange to shut down its session - // when it's done. - void AbortExchangesForFabricExceptOne(FabricIndex fabricIndex, ExchangeContext * deferred); - SessionManager * GetSessionManager() const { return mSessionManager; } ReliableMessageMgr * GetReliableMessageMgr() { return &mReliableMessageMgr; }; diff --git a/src/messaging/ReliableMessageContext.h b/src/messaging/ReliableMessageContext.h index 9b2f8d67ab0539..c3f6ae33793562 100644 --- a/src/messaging/ReliableMessageContext.h +++ b/src/messaging/ReliableMessageContext.h @@ -124,11 +124,6 @@ class ReliableMessageContext /// Determine whether this exchange is a EphemeralExchange for replying a StandaloneAck bool IsEphemeralExchange() const; - // If SetAutoReleaseSession() is called, this exchange must be using a SecureSession, and should - // evict it when the exchange is done with all its work (including any MRP traffic). - void SetAutoReleaseSession(); - bool ReleaseSessionOnDestruction(); - /** * Get the reliable message manager that corresponds to this reliable * message context. @@ -253,15 +248,5 @@ inline bool ReliableMessageContext::IsEphemeralExchange() const return mFlags.Has(Flags::kFlagEphemeralExchange); } -inline void ReliableMessageContext::SetAutoReleaseSession() -{ - mFlags.Set(Flags::kFlagAutoReleaseSession, true); -} - -inline bool ReliableMessageContext::ReleaseSessionOnDestruction() -{ - return mFlags.Has(Flags::kFlagAutoReleaseSession); -} - } // namespace Messaging } // namespace chip diff --git a/src/messaging/tests/BUILD.gn b/src/messaging/tests/BUILD.gn index 10f34fca6d2f45..4b7d0a2585ec2e 100644 --- a/src/messaging/tests/BUILD.gn +++ b/src/messaging/tests/BUILD.gn @@ -43,11 +43,13 @@ static_library("helpers") { chip_test_suite("tests") { output_name = "libMessagingLayerTests" - sources = [ "TestMessagingLayer.h" ] + test_sources = [] if (chip_device_platform != "efr32") { # TODO(#10447): ReliableMessage Test has HF, and ExchangeMgr hangs on EFR32. - sources += [ + # And TestAbortExchangesForFabric does not link on EFR32 for some reason. + test_sources += [ + "TestAbortExchangesForFabric.cpp", "TestExchangeMgr.cpp", "TestReliableMessageProtocol.cpp", ] @@ -67,9 +69,4 @@ chip_test_suite("tests") { "${nlio_root}:nlio", "${nlunit_test_root}:nlunit-test", ] - - tests = [ - "TestExchangeMgr", - "TestReliableMessageProtocol", - ] } diff --git a/src/messaging/tests/MessagingContext.h b/src/messaging/tests/MessagingContext.h index a759364de331e5..5ef51e1da99c6a 100644 --- a/src/messaging/tests/MessagingContext.h +++ b/src/messaging/tests/MessagingContext.h @@ -103,8 +103,6 @@ class MessagingContext : public PlatformMemoryUser static const uint16_t kAliceKeyId = 2; static const uint16_t kCharlieKeyId = 3; static const uint16_t kDavidKeyId = 4; - NodeId GetBobNodeId() const; - NodeId GetAliceNodeId() const; GroupId GetFriendsGroupId() const { return mFriendsGroupId; } SessionManager & GetSecureSessionManager() { return mSessionManager; } diff --git a/src/messaging/tests/TestAbortExchangesForFabric.cpp b/src/messaging/tests/TestAbortExchangesForFabric.cpp new file mode 100644 index 00000000000000..c83342b35e1fe2 --- /dev/null +++ b/src/messaging/tests/TestAbortExchangesForFabric.cpp @@ -0,0 +1,235 @@ +/* + * Copyright (c) 2022 Project CHIP Authors + * All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/** + * @file + * This file implements unit tests for aborting existing exchanges (except + * one) for a fabric. + */ + +#include +#include +#include +#include +#include +#include +#include +#include + +namespace { + +using namespace chip; +using namespace chip::Messaging; +using namespace chip::System; +using namespace chip::Protocols; + +using TestContext = Test::LoopbackMessagingContext; + +class MockAppDelegate : public ExchangeDelegate +{ +public: + CHIP_ERROR OnMessageReceived(ExchangeContext * ec, const PayloadHeader & payloadHeader, + System::PacketBufferHandle && buffer) override + { + mOnMessageReceivedCalled = true; + return CHIP_NO_ERROR; + } + + void OnResponseTimeout(ExchangeContext * ec) override {} + + bool mOnMessageReceivedCalled = false; +}; + +void CheckAbortAllButOneExchange(nlTestSuite * inSuite, void * inContext) +{ + TestContext & ctx = *reinterpret_cast(inContext); + + // We want to have two sessions using the same fabric id that we use for + // creating our exchange contexts. That lets us test exchanges on the same + // session as the "special exchange" as well as on other sessions. + auto & sessionManager = ctx.GetSecureSessionManager(); + + // Use key ids that are not going to collide with anything else that ctx is + // doing. + // TODO: These should really be CASE sessions... + SessionHolder session1; + CHIP_ERROR err = + sessionManager.InjectPaseSessionWithTestKey(session1, 100, ctx.GetBobFabric()->GetNodeId(), 101, ctx.GetAliceFabricIndex(), + ctx.GetBobAddress(), CryptoContext::SessionRole::kInitiator); + NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR); + + SessionHolder session1Reply; + err = sessionManager.InjectPaseSessionWithTestKey(session1Reply, 101, ctx.GetAliceFabric()->GetNodeId(), 100, + ctx.GetBobFabricIndex(), ctx.GetAliceAddress(), + CryptoContext::SessionRole::kResponder); + NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR); + + // TODO: Ideally this would go to a different peer, but we don't have that + // set up right now: only Alice and Bob have useful node ids and whatnot. + SessionHolder session2; + err = + sessionManager.InjectPaseSessionWithTestKey(session2, 200, ctx.GetBobFabric()->GetNodeId(), 201, ctx.GetAliceFabricIndex(), + ctx.GetBobAddress(), CryptoContext::SessionRole::kInitiator); + NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR); + + SessionHolder session2Reply; + err = sessionManager.InjectPaseSessionWithTestKey(session2Reply, 201, ctx.GetAliceFabric()->GetNodeId(), 200, + ctx.GetBobFabricIndex(), ctx.GetAliceAddress(), + CryptoContext::SessionRole::kResponder); + NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR); + + auto & exchangeMgr = ctx.GetExchangeManager(); + + MockAppDelegate delegate; + Echo::EchoServer server; + err = server.Init(&exchangeMgr); + NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR); + + auto & loopback = ctx.GetLoopback(); + + auto trySendMessage = [&](ExchangeContext * exchange, SendMessageFlags flags) { + PacketBufferHandle buffer = MessagePacketBuffer::New(0); + NL_TEST_ASSERT(inSuite, !buffer.IsNull()); + return exchange->SendMessage(Echo::MsgType::EchoRequest, std::move(buffer), flags); + }; + + auto sendAndDropMessage = [&](ExchangeContext * exchange, SendMessageFlags flags) { + // Send a message on the given exchange with the given flags, make sure + // it's not delivered. + loopback.mNumMessagesToDrop = 1; + loopback.mDroppedMessageCount = 0; + + err = trySendMessage(exchange, flags); + NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR); + + ctx.DrainAndServiceIO(); + NL_TEST_ASSERT(inSuite, !delegate.mOnMessageReceivedCalled); + NL_TEST_ASSERT(inSuite, loopback.mDroppedMessageCount == 1); + }; + + ReliableMessageMgr * rm = ctx.GetExchangeManager().GetReliableMessageMgr(); + + // We want to test three possible exchange states: + // 1) Closed but waiting for ack. + // 2) Waiting for a response. + // 3) Waiting for a send. + auto * waitingForAck1 = exchangeMgr.NewContext(session1.Get().Value(), &delegate); + NL_TEST_ASSERT(inSuite, waitingForAck1 != nullptr); + sendAndDropMessage(waitingForAck1, SendMessageFlags::kNone); + NL_TEST_ASSERT(inSuite, rm->TestGetCountRetransTable() == 1); + + auto * waitingForAck2 = exchangeMgr.NewContext(session2.Get().Value(), &delegate); + NL_TEST_ASSERT(inSuite, waitingForAck2 != nullptr); + sendAndDropMessage(waitingForAck2, SendMessageFlags::kNone); + NL_TEST_ASSERT(inSuite, rm->TestGetCountRetransTable() == 2); + + auto * waitingForIncomingMessage1 = exchangeMgr.NewContext(session1.Get().Value(), &delegate); + NL_TEST_ASSERT(inSuite, waitingForIncomingMessage1 != nullptr); + sendAndDropMessage(waitingForIncomingMessage1, SendMessageFlags::kExpectResponse); + NL_TEST_ASSERT(inSuite, rm->TestGetCountRetransTable() == 3); + + auto * waitingForIncomingMessage2 = exchangeMgr.NewContext(session2.Get().Value(), &delegate); + NL_TEST_ASSERT(inSuite, waitingForIncomingMessage2 != nullptr); + sendAndDropMessage(waitingForIncomingMessage2, SendMessageFlags::kExpectResponse); + NL_TEST_ASSERT(inSuite, rm->TestGetCountRetransTable() == 4); + + auto * waitingForSend1 = exchangeMgr.NewContext(session1.Get().Value(), &delegate); + NL_TEST_ASSERT(inSuite, waitingForSend1 != nullptr); + waitingForSend1->WillSendMessage(); + + auto * waitingForSend2 = exchangeMgr.NewContext(session2.Get().Value(), &delegate); + NL_TEST_ASSERT(inSuite, waitingForSend2 != nullptr); + waitingForSend2->WillSendMessage(); + + // Grab handles to our sessions now, before we evict things. + const auto & sessionHandle1 = session1.Get(); + const auto & sessionHandle2 = session2.Get(); + + NL_TEST_ASSERT(inSuite, session1); + NL_TEST_ASSERT(inSuite, session2); + auto * specialExhange = exchangeMgr.NewContext(session1.Get().Value(), &delegate); + specialExhange->AbortAllOtherCommunicationOnFabric(); + + NL_TEST_ASSERT(inSuite, rm->TestGetCountRetransTable() == 0); + NL_TEST_ASSERT(inSuite, !session1); + NL_TEST_ASSERT(inSuite, !session2); + + NL_TEST_ASSERT(inSuite, exchangeMgr.NewContext(sessionHandle1.Value(), &delegate) == nullptr); + NL_TEST_ASSERT(inSuite, exchangeMgr.NewContext(sessionHandle2.Value(), &delegate) == nullptr); + + // Make sure we can't send messages on any of the other exchanges. + NL_TEST_ASSERT(inSuite, trySendMessage(waitingForSend1, SendMessageFlags::kExpectResponse) != CHIP_NO_ERROR); + NL_TEST_ASSERT(inSuite, trySendMessage(waitingForSend2, SendMessageFlags::kExpectResponse) != CHIP_NO_ERROR); + + // Make sure we can send a message on the special exchange. + NL_TEST_ASSERT(inSuite, !delegate.mOnMessageReceivedCalled); + err = trySendMessage(specialExhange, SendMessageFlags::kNone); + NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR); + // Should be waiting for an ack now. + NL_TEST_ASSERT(inSuite, rm->TestGetCountRetransTable() == 1); + + ctx.DrainAndServiceIO(); + + // Should not get an app-level response, since we are not expecting one. + NL_TEST_ASSERT(inSuite, !delegate.mOnMessageReceivedCalled); + // We should have gotten our ack. + NL_TEST_ASSERT(inSuite, rm->TestGetCountRetransTable() == 0); + + waitingForSend1->Close(); + waitingForSend2->Close(); +} + +// Test Suite + +/** + * Test Suite that lists all the test functions. + */ +// clang-format off +const nlTest sTests[] = +{ + NL_TEST_DEF("Test aborting all but one exchange", CheckAbortAllButOneExchange), + + NL_TEST_SENTINEL() +}; +// clang-format on + +// clang-format off +nlTestSuite sSuite = +{ + "Test-AbortExchangesForFabric", + &sTests[0], + TestContext::Initialize, + TestContext::Finalize +}; +// clang-format on + +} // anonymous namespace + +/** + * Main + */ +int TestAbortExchangesForFabric() +{ + TestContext sContext; + + // Run test suit against one context + nlTestRunner(&sSuite, &sContext); + + return (nlTestRunnerStats(&sSuite)); +} + +CHIP_REGISTER_TEST_SUITE(TestAbortExchangesForFabric); diff --git a/src/messaging/tests/TestExchangeMgr.cpp b/src/messaging/tests/TestExchangeMgr.cpp index 39d14dab1477cc..ff9d56929f124b 100644 --- a/src/messaging/tests/TestExchangeMgr.cpp +++ b/src/messaging/tests/TestExchangeMgr.cpp @@ -21,8 +21,6 @@ * This file implements unit tests for the ExchangeManager implementation. */ -#include "TestMessagingLayer.h" - #include #include #include diff --git a/src/messaging/tests/TestExchangeMgrDriver.cpp b/src/messaging/tests/TestExchangeMgrDriver.cpp deleted file mode 100644 index c4699e8d27bb50..00000000000000 --- a/src/messaging/tests/TestExchangeMgrDriver.cpp +++ /dev/null @@ -1,35 +0,0 @@ -/* - * - * Copyright (c) 2020 Project CHIP Authors - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -/** - * @file - * This file implements a standalone/native program executable - * test driver for the CHIP core library CHIP ExchangeManager tests. - * - */ - -#include "TestMessagingLayer.h" - -#include - -int main() -{ - // Generate machine-readable, comma-separated value (CSV) output. - nlTestSetOutputStyle(OUTPUT_CSV); - - return (TestExchangeMgr()); -} diff --git a/src/messaging/tests/TestMessagingLayer.h b/src/messaging/tests/TestMessagingLayer.h deleted file mode 100644 index 7bdcef314c4c0b..00000000000000 --- a/src/messaging/tests/TestMessagingLayer.h +++ /dev/null @@ -1,36 +0,0 @@ -/* - * - * Copyright (c) 2020 Project CHIP Authors - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -/** - * @file - * This file declares test entry points for CHIP Messaging layer - * layer library unit tests. - * - */ - -#pragma once - -#ifdef __cplusplus -extern "C" { -#endif - -int TestExchangeMgr(void); -int TestReliableMessageProtocol(void); - -#ifdef __cplusplus -} -#endif diff --git a/src/messaging/tests/TestReliableMessageProtocol.cpp b/src/messaging/tests/TestReliableMessageProtocol.cpp index 4ddf2c63919759..a0c204c919366e 100644 --- a/src/messaging/tests/TestReliableMessageProtocol.cpp +++ b/src/messaging/tests/TestReliableMessageProtocol.cpp @@ -22,8 +22,6 @@ * implementation. */ -#include "TestMessagingLayer.h" - #include #include #include diff --git a/src/messaging/tests/TestReliableMessageProtocolDriver.cpp b/src/messaging/tests/TestReliableMessageProtocolDriver.cpp deleted file mode 100644 index b7cac4537e3f62..00000000000000 --- a/src/messaging/tests/TestReliableMessageProtocolDriver.cpp +++ /dev/null @@ -1,35 +0,0 @@ -/* - * - * Copyright (c) 2020 Project CHIP Authors - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -/** - * @file - * This file implements a standalone/native program executable test driver - * for the CHIP core library CHIP ReliableMessageProtocol tests. - * - */ - -#include "TestMessagingLayer.h" - -#include - -int main() -{ - // Generate machine-readable, comma-separated value (CSV) output. - nlTestSetOutputStyle(OUTPUT_CSV); - - return (TestReliableMessageProtocol()); -} diff --git a/src/transport/SecureSession.cpp b/src/transport/SecureSession.cpp index 9363d92b323f21..598c2f98b3f782 100644 --- a/src/transport/SecureSession.cpp +++ b/src/transport/SecureSession.cpp @@ -50,7 +50,7 @@ void SecureSession::MarkForRemoval() } } -void SecureSession::MarkInactive() +void SecureSession::MarkInactive(SessionHolder & deferred) { ChipLogDetail(Inet, "SecureSession[%p]: MarkInactive Type:%d LSID:%d", this, to_underlying(mSecureSessionType), mLocalSessionId); @@ -61,12 +61,18 @@ void SecureSession::MarkInactive() VerifyOrDie(false); return; case State::kActive: + // Make sure we don't notify "deferred" that we're being removed. This + // ensures that "deferred" is in fact in our holder list. + RemoveHolder(deferred); // By setting this state, IsActiveSession() will return false, which prevents creating new exchanges. mState = State::kInactive; + NotifySessionReleased(); + // Add the holder back. + AddHolder(deferred); return; case State::kInactive: case State::kPendingRemoval: - // Do nothing + VerifyOrDie(false); return; } } diff --git a/src/transport/SecureSession.h b/src/transport/SecureSession.h index 829b179965a8b2..b266300564faa4 100644 --- a/src/transport/SecureSession.h +++ b/src/transport/SecureSession.h @@ -144,8 +144,12 @@ class SecureSession : public Session, public ReferenceCountedGetPeer().GetFabricIndex() == fabricIndex) { - if (session == deferredSecureSession) - session->MarkInactive(); - else + if (session != deferredSecureSession) + { session->MarkForRemoval(); + } } return Loop::Continue; }); diff --git a/src/transport/SessionManager.h b/src/transport/SessionManager.h index d88031f670f4ac..d5c843bc0fbb67 100644 --- a/src/transport/SessionManager.h +++ b/src/transport/SessionManager.h @@ -177,8 +177,8 @@ class DLL_EXPORT SessionManager : public TransportMgrDelegate void ExpireAllPASEPairings(); // This API is used by commands that need to release all existing sessions on a fabric but need to make sure the response to the - // command still goes out on the exchange the command came in on. This API flags that the release of the session used by the - // exchange is deferred until the exchange is done. + // command still goes out on the exchange the command came in on. This API + // relies on the caller to handle releasing the "deferred" session. void ReleaseSessionsForFabricExceptOne(FabricIndex fabricIndex, const SessionHandle & deferred); /**