diff --git a/src/messaging/ExchangeContext.cpp b/src/messaging/ExchangeContext.cpp index b3bc08899ebfbb..1e66101304a611 100644 --- a/src/messaging/ExchangeContext.cpp +++ b/src/messaging/ExchangeContext.cpp @@ -325,9 +325,6 @@ ExchangeContext::~ExchangeContext() VerifyOrDie(mExchangeMgr != nullptr && GetReferenceCount() == 0); VerifyOrDie(!IsAckPending()); - if (ReleaseSessionOnDestruction() && mSession) - mSession->AsSecureSession()->MarkForEviction(); - #if CONFIG_DEVICE_LAYER && CHIP_DEVICE_CONFIG_ENABLE_SED // Make sure that the exchange withdraws the request for Sleepy End Device active mode. UpdateSEDIntervalMode(false); @@ -371,6 +368,11 @@ bool ExchangeContext::MatchExchange(const SessionHandle & session, const PacketH void ExchangeContext::OnSessionReleased() { + if (ShouldIgnoreSessionRelease()) + { + return; + } + if (mFlags.Has(Flags::kFlagClosed)) { // Exchange is already being closed. It may occur when closing an exchange after sending @@ -573,5 +575,31 @@ ExchangeMessageDispatch & ExchangeContext::GetMessageDispatch(bool isEphemeralEx return ApplicationExchangeDispatch::Instance(); } +void ExchangeContext::AbortAllOtherCommunicationOnFabric() +{ + if (!mSession || !mSession->IsSecureSession()) + { + ChipLogError(ExchangeManager, "AbortAllOtherCommunicationOnFabric called when we don't have a PASE/CASE session"); + return; + } + + // Save our session so it does not actually go away. + Optional session = mSession.Get(); + + SetIgnoreSessionRelease(true); + + GetExchangeMgr()->GetSessionManager()->ExpireAllPairingsForFabric(mSession->GetFabricIndex()); + + mSession.GrabExpiredSession(session.Value()); + + SetIgnoreSessionRelease(false); +} + +void ExchangeContext::ExchangeSessionHolder::GrabExpiredSession(const SessionHandle & session) +{ + VerifyOrDie(session->AsSecureSession()->IsPendingEviction()); + GrabUnchecked(session); +} + } // namespace Messaging } // namespace chip diff --git a/src/messaging/ExchangeContext.h b/src/messaging/ExchangeContext.h index b59c68cba17fb1..c52ba48630a726 100644 --- a/src/messaging/ExchangeContext.h +++ b/src/messaging/ExchangeContext.h @@ -184,15 +184,33 @@ 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: + class ExchangeSessionHolder : public SessionHolderWithDelegate + { + public: + ExchangeSessionHolder(ExchangeContext & exchange) : SessionHolderWithDelegate(exchange) {} + void GrabExpiredSession(const SessionHandle & session); + }; + Timeout mResponseTimeout{ 0 }; // Maximum time to wait for response (in milliseconds); 0 disables response timeout. ExchangeDelegate * mDelegate = nullptr; ExchangeManager * mExchangeMgr = nullptr; ExchangeMessageDispatch & mDispatch; - SessionHolderWithDelegate mSession; // The connection state - uint16_t mExchangeId; // Assigned exchange ID. + ExchangeSessionHolder mSession; // The connection state + uint16_t mExchangeId; // Assigned exchange ID. /** * Determine whether a response is currently expected for a message that was sent over @@ -274,7 +292,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 SetIgnoreSessionRelease(bool ignore); + inline bool ShouldIgnoreSessionRelease(); }; +inline void ExchangeContext::SetIgnoreSessionRelease(bool ignore) +{ + mFlags.Set(Flags::kFlagIgnoreSessionRelease, ignore); +} + +inline bool ExchangeContext::ShouldIgnoreSessionRelease() +{ + return mFlags.Has(Flags::kFlagIgnoreSessionRelease); +} + } // namespace Messaging } // namespace chip diff --git a/src/messaging/ExchangeMgr.cpp b/src/messaging/ExchangeMgr.cpp index c1d613d4aa39de..fcdcfeeecd5058 100644 --- a/src/messaging/ExchangeMgr.cpp +++ b/src/messaging/ExchangeMgr.cpp @@ -236,7 +236,14 @@ void ExchangeManager::OnMessageReceived(const PacketHeader & packetHeader, const packetHeader.GetDestinationGroupId().Value()); } - // Do not handle unsolicited messages on a inactive session. + // Do not handle messages that don't match an existing exchange on an + // inactive session, since we should not be creating new exchanges there. + if (!session->IsActiveSession()) + { + ChipLogProgress(ExchangeManager, "Dropping message on inactive session that does not match an existing exchange"); + return; + } + // If it's not a duplicate message, search for an unsolicited message handler if it is marked as being sent by an initiator. // Since we didn't find an existing exchange that matches the message, it must be an unsolicited message. However all // unsolicited messages must be marked as being from an initiator. @@ -376,23 +383,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..886ed5c3ba341b 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. @@ -170,8 +165,8 @@ class ReliableMessageContext /// When set, signifies that the exchange created sorely for replying a StandaloneAck kFlagEphemeralExchange = (1u << 9), - /// When set, automatically release the session when this exchange is destroyed. - kFlagAutoReleaseSession = (1u << 10), + /// When set, ignore session being released, because we are releasing it ourselves. + kFlagIgnoreSessionRelease = (1u << 10), }; BitFlags mFlags; // Internal state flags @@ -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 52ecc256ecca94..05aa6cddbd63e4 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 5af68d5fd35504..adb1db94872033 100644 --- a/src/transport/SecureSession.cpp +++ b/src/transport/SecureSession.cpp @@ -117,11 +117,6 @@ void SecureSession::MarkAsDefunct() // return; - case State::kInactive: - // - // Once a session is marked Inactive, we CANNOT bring it back to either being active or defunct. - // - FALLTHROUGH; case State::kPendingEviction: // // Once a session is headed for eviction, we CANNOT bring it back to either being active or defunct. @@ -148,8 +143,6 @@ void SecureSession::MarkForEviction() case State::kDefunct: FALLTHROUGH; case State::kActive: - FALLTHROUGH; - case State::kInactive: Release(); // Decrease the ref which is retained at Activate MoveToState(State::kPendingEviction); NotifySessionReleased(); @@ -161,29 +154,6 @@ void SecureSession::MarkForEviction() } } -void SecureSession::MarkInactive() -{ - ChipLogDetail(Inet, "SecureSession[%p]: MarkInactive Type:%d LSID:%d", this, to_underlying(mSecureSessionType), - mLocalSessionId); - ReferenceCountedHandle ref(*this); - switch (mState) - { - case State::kEstablishing: - VerifyOrDie(false); - return; - case State::kDefunct: - FALLTHROUGH; - case State::kActive: - // By setting this state, IsActiveSession() will return false, which prevents creating new exchanges. - mState = State::kInactive; - return; - case State::kInactive: - case State::kPendingEviction: - // Do nothing - return; - } -} - Access::SubjectDescriptor SecureSession::GetSubjectDescriptor() const { Access::SubjectDescriptor subjectDescriptor; diff --git a/src/transport/SecureSession.h b/src/transport/SecureSession.h index 6c68e9bc72211a..8f026ad83009a6 100644 --- a/src/transport/SecureSession.h +++ b/src/transport/SecureSession.h @@ -147,9 +147,6 @@ class SecureSession : public Session, public ReferenceCountedAsSecureSession()->IsEstablishing()) return false; - mSession.Emplace(session.mSession); - session->AddHolder(*this); + GrabUnchecked(session); return true; } @@ -96,9 +95,15 @@ bool SessionHolder::Grab(const SessionHandle & session) if (!session->IsActiveSession()) return false; + GrabUnchecked(session); + return true; +} + +void SessionHolder::GrabUnchecked(const SessionHandle & session) +{ + VerifyOrDie(!mSession.HasValue()); mSession.Emplace(session.mSession); session->AddHolder(*this); - return true; } void SessionHolder::Release() diff --git a/src/transport/SessionHolder.h b/src/transport/SessionHolder.h index 2886987685940f..8d830a267d763d 100644 --- a/src/transport/SessionHolder.h +++ b/src/transport/SessionHolder.h @@ -73,7 +73,10 @@ class SessionHolder : public IntrusiveListNodeBase<> // There is not delegate, nothing to do here virtual void DispatchSessionEvent(SessionDelegate::Event event) {} -private: +protected: + // Helper for use by the Grab methods. + void GrabUnchecked(const SessionHandle & session); + Optional> mSession; }; diff --git a/src/transport/SessionManager.cpp b/src/transport/SessionManager.cpp index 5be749e54393f8..db37077fa3869a 100644 --- a/src/transport/SessionManager.cpp +++ b/src/transport/SessionManager.cpp @@ -368,23 +368,6 @@ void SessionManager::ExpireAllPASEPairings() }); } -void SessionManager::ReleaseSessionsForFabricExceptOne(FabricIndex fabricIndex, const SessionHandle & deferred) -{ - VerifyOrDie(deferred->IsSecureSession()); - SecureSession * deferredSecureSession = deferred->AsSecureSession(); - - mSecureSessions.ForEachSession([&](auto session) { - if (session->GetPeer().GetFabricIndex() == fabricIndex) - { - if (session == deferredSecureSession) - session->MarkInactive(); - else - session->MarkForEviction(); - } - return Loop::Continue; - }); -} - Optional SessionManager::AllocateSession(SecureSession::Type secureSessionType, const ScopedNodeId & sessionEvictionHint) { @@ -553,7 +536,13 @@ void SessionManager::SecureUnicastMessageDispatch(const PacketHeader & packetHea Transport::SecureSession * secureSession = session.Value()->AsSecureSession(); - if (!secureSession->IsDefunct() && !secureSession->IsActiveSession()) + // We need to allow through messages even on sessions that are pending + // evictions, because for some cases (UpdateNOC, RemoveFabric, etc) there + // can be a single exchange alive on the session waiting for a MRP ack, and + // we need to make sure to send the ack through. The exchange manager is + // responsible for ensuring that such messages do not lead to new exchange + // creation. + if (!secureSession->IsDefunct() && !secureSession->IsActiveSession() && !secureSession->IsPendingEviction()) { ChipLogError(Inet, "Secure transport received message on a session in an invalid state (state = '%s')", secureSession->GetStateStr()); diff --git a/src/transport/SessionManager.h b/src/transport/SessionManager.h index 13e06fae8797d7..219fd4eb20c069 100644 --- a/src/transport/SessionManager.h +++ b/src/transport/SessionManager.h @@ -178,11 +178,6 @@ class DLL_EXPORT SessionManager : public TransportMgrDelegate void ExpireAllPairingsForFabric(FabricIndex fabric); 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. - void ReleaseSessionsForFabricExceptOne(FabricIndex fabricIndex, const SessionHandle & deferred); - /** * @brief * Return the System Layer pointer used by current SessionManager.