diff --git a/src/messaging/ExchangeMgr.cpp b/src/messaging/ExchangeMgr.cpp index cd258b8afe8db4..fc5bea10b83e22 100644 --- a/src/messaging/ExchangeMgr.cpp +++ b/src/messaging/ExchangeMgr.cpp @@ -111,8 +111,12 @@ CHIP_ERROR ExchangeManager::Shutdown() ExchangeContext * ExchangeManager::NewContext(const SessionHandle & session, ExchangeDelegate * delegate) { - // Disallow creating exchange on an inactive session - VerifyOrReturnError(session->IsActiveSession(), nullptr); + if (!session->IsActiveSession()) + { + // Disallow creating exchange on an inactive session + ChipLogError(ExchangeManager, "NewContext failed: session inactive"); + return nullptr; + } return mContextPool.CreateObject(this, mNextExchangeId++, session, true, delegate); } diff --git a/src/protocols/secure_channel/PairingSession.cpp b/src/protocols/secure_channel/PairingSession.cpp index 55d992c1655d83..806d0ec4306785 100644 --- a/src/protocols/secure_channel/PairingSession.cpp +++ b/src/protocols/secure_channel/PairingSession.cpp @@ -27,7 +27,7 @@ CHIP_ERROR PairingSession::AllocateSecureSession(SessionManager & sessionManager { auto handle = sessionManager.AllocateSession(GetSecureSessionType()); VerifyOrReturnError(handle.HasValue(), CHIP_ERROR_NO_MEMORY); - VerifyOrReturnError(mSecureSessionHolder.GrabPairing(handle.Value()), CHIP_ERROR_INTERNAL); + VerifyOrReturnError(mSecureSessionHolder.GrabPairingSession(handle.Value()), CHIP_ERROR_INTERNAL); mSessionManager = &sessionManager; return CHIP_NO_ERROR; } diff --git a/src/transport/SecureSession.h b/src/transport/SecureSession.h index 47bee45412f0ca..a505a3237f00eb 100644 --- a/src/transport/SecureSession.h +++ b/src/transport/SecureSession.h @@ -66,6 +66,7 @@ class SecureSession : public Session, public ReferenceCountedIsActiveSession can be used to check if the session is active. + * SessionHandle is reference counted such that it is never dangling, but there can be a gray period when the session is marked + * as pending removal but not actually removed yet. During this period, the handle is functional, but the underlying session + * won't be able to be grabbed by any SessionHolder. SessionHandle->IsActiveSession can be used to check if the session is + * active. */ class SessionHandle { diff --git a/src/transport/SessionHolder.cpp b/src/transport/SessionHolder.cpp index f9420a88e7a306..97498cf9dc41fb 100644 --- a/src/transport/SessionHolder.cpp +++ b/src/transport/SessionHolder.cpp @@ -74,7 +74,7 @@ SessionHolder & SessionHolder::operator=(SessionHolder && that) return *this; } -bool SessionHolder::GrabPairing(const SessionHandle & session) +bool SessionHolder::GrabPairingSession(const SessionHandle & session) { Release(); diff --git a/src/transport/SessionHolder.h b/src/transport/SessionHolder.h index ac7cc507b8602a..7e0433a6c345ea 100644 --- a/src/transport/SessionHolder.h +++ b/src/transport/SessionHolder.h @@ -47,7 +47,7 @@ class SessionHolder : public SessionDelegate, public IntrusiveListNodeBase return mSession.HasValue() && &mSession.Value().Get() == &session.mSession.Get(); } - bool GrabPairing(const SessionHandle & session); // Should be only used inside CASE/PASE pairing. + bool GrabPairingSession(const SessionHandle & session); // Should be only used inside CASE/PASE pairing. bool Grab(const SessionHandle & session); void Release(); diff --git a/src/transport/tests/TestSessionManager.cpp b/src/transport/tests/TestSessionManager.cpp index cc9179a92b2330..d3d60855879bd9 100644 --- a/src/transport/tests/TestSessionManager.cpp +++ b/src/transport/tests/TestSessionManager.cpp @@ -672,7 +672,7 @@ void SessionAllocationTest(nlTestSuite * inSuite, void * inContext) auto handle = sessionManager.AllocateSession(Transport::SecureSession::Type::kPASE); NL_TEST_ASSERT(inSuite, handle.HasValue()); SessionHolder session; - session.GrabPairing(handle.Value()); + session.GrabPairingSession(handle.Value()); sessionId1 = session->AsSecureSession()->GetLocalSessionId(); }