diff --git a/src/transport/BUILD.gn b/src/transport/BUILD.gn index defde06d387fdc..ff8afecd0db5bc 100644 --- a/src/transport/BUILD.gn +++ b/src/transport/BUILD.gn @@ -47,6 +47,7 @@ static_library("transport") { "SessionManager.h", "SessionMessageCounter.h", "SessionMessageDelegate.h", + "SessionSharedPtr.h", "TransportMgr.h", "TransportMgrBase.cpp", "TransportMgrBase.h", diff --git a/src/transport/SessionHandle.h b/src/transport/SessionHandle.h index 344dab3580b0ec..7cef903637706c 100644 --- a/src/transport/SessionHandle.h +++ b/src/transport/SessionHandle.h @@ -18,7 +18,9 @@ #pragma once #include +#include #include +#include namespace chip { @@ -29,13 +31,15 @@ class Session; class SessionHolder; /** @brief - * Non-copyable session reference. All SessionHandles are created within SessionManager. SessionHandle is not - * reference *counted, hence it is not allowed to store SessionHandle anywhere except for function arguments and - * return values. SessionHandle is short-lived as it is only available as stack variable, so it is never dangling. */ -class SessionHandle + * A non-copyable version of SessionSharedPtr that is always guaranteed to point to a valid Session. This is always + * created within SessionManager. + * + * Since this is a strong reference, this should be short-lived and only used as a stack variable. + */ +class SessionHandle : protected SessionSharedPtr { public: - SessionHandle(Transport::Session & session) : mSession(session) {} + SessionHandle(Transport::Session & session) : SessionSharedPtr(session) {} ~SessionHandle() {} SessionHandle(const SessionHandle &) = delete; @@ -43,13 +47,13 @@ class SessionHandle SessionHandle(SessionHandle &&) = default; SessionHandle & operator=(SessionHandle &&) = delete; - bool operator==(const SessionHandle & that) const { return &mSession.Get() == &that.mSession.Get(); } + bool operator==(const SessionHandle & that) const { return Get() == that.Get(); } - Transport::Session * operator->() const { return mSession.operator->(); } + Transport::Session * operator->() const { return SessionSharedPtr::operator->(); } private: + // TODO: Remove this once SessionHolder pivots to truly being a weak reference (#18399). friend class SessionHolder; - ReferenceCountedHandle mSession; }; } // namespace chip diff --git a/src/transport/SessionHolder.cpp b/src/transport/SessionHolder.cpp index 290cc1803fafcb..6fdbf2e2962cc8 100644 --- a/src/transport/SessionHolder.cpp +++ b/src/transport/SessionHolder.cpp @@ -75,7 +75,7 @@ SessionHolder & SessionHolder::operator=(SessionHolder && that) void SessionHolder::Grab(const SessionHandle & session) { Release(); - mSession.Emplace(session.mSession); + mSession.Emplace(session.mSession.Value()); session->AddHolder(*this); } diff --git a/src/transport/SessionHolder.h b/src/transport/SessionHolder.h index 28321d9e71c2e9..11e5f1baa43e0d 100644 --- a/src/transport/SessionHolder.h +++ b/src/transport/SessionHolder.h @@ -44,7 +44,8 @@ class SessionHolder : public SessionDelegate, public IntrusiveListNodeBase bool Contains(const SessionHandle & session) const { - return mSession.HasValue() && &mSession.Value().Get() == &session.mSession.Get(); + auto ourSession = ToOptional(); + return ourSession.HasValue() && ourSession.Value() == session; } void Grab(const SessionHandle & session); diff --git a/src/transport/SessionSharedPtr.h b/src/transport/SessionSharedPtr.h new file mode 100644 index 00000000000000..6d418224f5b19c --- /dev/null +++ b/src/transport/SessionSharedPtr.h @@ -0,0 +1,105 @@ +/* + * + * Copyright (c) 2021 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. + */ + +#pragma once + +#include +#include +#include + +namespace chip { + +namespace Transport { +class Session; +} // namespace Transport + +class SessionHolder; + +/** @brief + * A shared_ptr like smart pointer to manage the lifetime of a Session object. Like a shared_ptr, + * this object can start out not tracking any Session and be attached to a Session there-after. + * The underlying Session is guaranteed to remain active and resident until all references to it from SessionSharedPtr + * instances in the system have gone away, at which point it invokes its custom destructor. + * + * Just because a Session is refcounted does not mean it actually gets destroyed upon reaching a count of 0. + * UnauthenticatedSession and SecureSession have different logic that gets invoked when the count hits 0. + * + * This should really only be used during session setup by the entity setting up the session. + * Once setup, the session should transfer ownership to the SessionManager at which point, + * all clients in the system should only be holding SessionWeakPtrs (SessionWeakPtr doesn't exist yet, but once + * #18399 is complete, SessionHolder will become SessionWeakPtr). + * + * This is copy-constructible. + */ +class SessionSharedPtr +{ +public: + SessionSharedPtr() {} + SessionSharedPtr(Transport::Session & session) { mSession.Emplace(session); } + + SessionSharedPtr(const SessionSharedPtr & session) { Grab(session); } + + /* + * Add ourselves as a shared owner on the passed in session (if it points to + * a valid session). The passed in + * session object can now be free'ed safely without impacting the underlying + * Session. + */ + void operator=(const SessionSharedPtr & session) { Grab(session); } + + /* + * If we're currently pointing to a valid session, remove ourselves + * as a shared owner of that session. If there are no more shared owners + * on that session, that session MAY be reclaimed. + */ + void Release() { mSession.ClearValue(); } + + /* + * Assume shared ownership of the provided session. + */ + void Grab(Transport::Session & session) { mSession.Emplace(session); } + + Transport::Session * operator->() const { return Get(); } + +protected: + Transport::Session * Get() const + { + if (mSession.HasValue()) + { + return &mSession.Value().Get(); + } + + return nullptr; + } + +private: + void Grab(const SessionSharedPtr & session) + { + auto * underlyingSession = session.Get(); + if (underlyingSession) + { + mSession.Emplace(*underlyingSession); + } + } + + Optional> mSession; + + // TODO: Remove this once SessionHolder pivots to truly being a weak reference (#18399). + friend class SessionHolder; +}; + +} // namespace chip