Fix compilation of transport/SessionHandle.h (#21765)

Currently SessionHandle, SessionHolder, and Session must be defined in order. Any other order will not compile. Merge these headers in order to satisfy the following objectives: 1) All headers compile in isolation 2) Header order inclusion does not matter This is the simplest way to fix the problem, and seems to be the only way to fix it without changing the class definitions.
project-chip · Feb 16, 2024 · f157460 · f157460
1 parent 316bcce
commit f157460
Show file tree

Hide file tree

Showing 4 changed files with 127 additions and 143 deletions.
diff --git a/src/transport/Session.h b/src/transport/Session.h
@@ -16,16 +16,140 @@
 
 #pragma once
 
+#include <access/SubjectDescriptor.h>
 #include <credentials/FabricTable.h>
 #include <lib/core/CHIPConfig.h>
+#include <lib/core/Optional.h>
 #include <lib/core/PeerId.h>
 #include <lib/core/ScopedNodeId.h>
+#include <lib/support/IntrusiveList.h>
+#include <lib/support/ReferenceCountedHandle.h>
 #include <messaging/ReliableMessageProtocolConfig.h>
 #include <platform/LockTracker.h>
-#include <transport/SessionHolder.h>
+#include <transport/SessionDelegate.h>
 #include <transport/raw/PeerAddress.h>
 
 namespace chip {
+namespace Transport {
+class Session;
+} // namespace Transport
+
+/** @brief
+ *    Non-copyable session reference. All SessionHandles are created within SessionManager. It is not allowed to store SessionHandle
+ *    anywhere except for function arguments and return values.
+ *
+ *    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
+{
+public:
+    SessionHandle(Transport::Session & session) : mSession(session) {}
+    ~SessionHandle() {}
+
+    SessionHandle(const SessionHandle &) = delete;
+    SessionHandle operator=(const SessionHandle &) = delete;
+    SessionHandle(SessionHandle &&)                = default;
+    SessionHandle & operator=(SessionHandle &&) = delete;
+
+    bool operator==(const SessionHandle & that) const { return &mSession.Get() == &that.mSession.Get(); }
+
+    Transport::Session * operator->() const { return mSession.operator->(); }
+
+private:
+    friend class SessionHolder;
+    ReferenceCountedHandle<Transport::Session> mSession;
+};
+
+/** @brief
+ *    Managed session reference. The object is used to store a session, the stored session will be automatically
+ *    released when the underlying session is released. One must verify it is available before use. The object can be
+ *    created using SessionHandle.Grab()
+ */
+class SessionHolder : public IntrusiveListNodeBase<>
+{
+public:
+    SessionHolder() {}
+    SessionHolder(const SessionHandle & handle) { Grab(handle); }
+    virtual ~SessionHolder();
+
+    SessionHolder(const SessionHolder &);
+    SessionHolder(SessionHolder && that);
+    SessionHolder & operator=(const SessionHolder &);
+    SessionHolder & operator=(SessionHolder && that);
+
+    virtual void SessionReleased() { Release(); }
+    virtual void ShiftToSession(const SessionHandle & session)
+    {
+        Release();
+        Grab(session);
+    }
+
+    bool Contains(const SessionHandle & session) const
+    {
+        return mSession.HasValue() && &mSession.Value().Get() == &session.mSession.Get();
+    }
+
+    bool GrabPairingSession(const SessionHandle & session); // Should be only used inside CASE/PASE pairing.
+    bool Grab(const SessionHandle & session);
+    void Release();
+
+    explicit operator bool() const { return mSession.HasValue(); }
+    Optional<SessionHandle> Get() const
+    {
+        //
+        // We cannot return mSession directly even if Optional<SessionHandle> is internally composed of the same bits,
+        // since they are not actually equivalent type-wise, and SessionHandle does not permit copy-construction.
+        //
+        // So, construct a new Optional<SessionHandle> from the underlying Transport::Session reference.
+        //
+        return mSession.HasValue() ? chip::MakeOptional<SessionHandle>(mSession.Value().Get())
+                                   : chip::Optional<SessionHandle>::Missing();
+    }
+
+    Transport::Session * operator->() const { return &mSession.Value().Get(); }
+
+    // There is not delegate, nothing to do here
+    virtual void DispatchSessionEvent(SessionDelegate::Event event) {}
+
+protected:
+    // Helper for use by the Grab methods.
+    void GrabUnchecked(const SessionHandle & session);
+
+    Optional<ReferenceCountedHandle<Transport::Session>> mSession;
+};
+
+/// @brief Extends SessionHolder to allow propagate SessionDelegate::* events to a given destination
+class SessionHolderWithDelegate : public SessionHolder
+{
+public:
+    SessionHolderWithDelegate(SessionDelegate & delegate) : mDelegate(delegate) {}
+    SessionHolderWithDelegate(const SessionHandle & handle, SessionDelegate & delegate) : SessionHolder(handle), mDelegate(delegate)
+    {}
+    operator bool() const { return SessionHolder::operator bool(); }
+
+    void SessionReleased() override
+    {
+        Release();
+
+        // Note, the session is already cleared during mDelegate.OnSessionReleased
+        mDelegate.OnSessionReleased();
+    }
+
+    void ShiftToSession(const SessionHandle & session) override
+    {
+        if (mDelegate.GetNewSessionHandlingPolicy() == SessionDelegate::NewSessionHandlingPolicy::kShiftToNewSession)
+            SessionHolder::ShiftToSession(session);
+    }
+
+    void DispatchSessionEvent(SessionDelegate::Event event) override { (mDelegate.*event)(); }
+
+private:
+    SessionDelegate & mDelegate;
+};
+
 namespace Transport {
 
 class SecureSession;

diff --git a/src/transport/SessionHandle.cpp b/src/transport/SessionHandle.cpp
@@ -15,12 +15,4 @@
  *    limitations under the License.
  */
 
-#include <transport/SecureSession.h>
 #include <transport/SessionHandle.h>
-#include <transport/SessionManager.h>
-
-namespace chip {
-
-using namespace Transport;
-
-} // namespace chip
diff --git a/src/transport/SessionHandle.h b/src/transport/SessionHandle.h
@@ -17,42 +17,4 @@
 
 #pragma once
 
-#include <access/SubjectDescriptor.h>
-#include <lib/support/ReferenceCountedHandle.h>
-
-namespace chip {
-
-namespace Transport {
-class Session;
-} // namespace Transport
-
-/** @brief
- *    Non-copyable session reference. All SessionHandles are created within SessionManager. It is not allowed to store SessionHandle
- *    anywhere except for function arguments and return values.
- *
- *    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
-{
-public:
-    SessionHandle(Transport::Session & session) : mSession(session) {}
-    ~SessionHandle() {}
-
-    SessionHandle(const SessionHandle &) = delete;
-    SessionHandle operator=(const SessionHandle &) = delete;
-    SessionHandle(SessionHandle &&)                = default;
-    SessionHandle & operator=(SessionHandle &&) = delete;
-
-    bool operator==(const SessionHandle & that) const { return &mSession.Get() == &that.mSession.Get(); }
-
-    Transport::Session * operator->() const { return mSession.operator->(); }
-
-private:
-    friend class SessionHolder;
-    ReferenceCountedHandle<Transport::Session> mSession;
-};
-
-} // namespace chip
+#include <transport/Session.h>
diff --git a/src/transport/SessionHolder.h b/src/transport/SessionHolder.h
@@ -16,98 +16,4 @@
 
 #pragma once
 
-#include <lib/core/Optional.h>
-#include <lib/support/IntrusiveList.h>
-#include <transport/SessionDelegate.h>
-#include <transport/SessionHandle.h>
-
-namespace chip {
-
-/** @brief
- *    Managed session reference. The object is used to store a session, the stored session will be automatically
- *    released when the underlying session is released. One must verify it is available before use. The object can be
- *    created using SessionHandle.Grab()
- */
-class SessionHolder : public IntrusiveListNodeBase<>
-{
-public:
-    SessionHolder() {}
-    SessionHolder(const SessionHandle & handle) { Grab(handle); }
-    virtual ~SessionHolder();
-
-    SessionHolder(const SessionHolder &);
-    SessionHolder(SessionHolder && that);
-    SessionHolder & operator=(const SessionHolder &);
-    SessionHolder & operator=(SessionHolder && that);
-
-    virtual void SessionReleased() { Release(); }
-    virtual void ShiftToSession(const SessionHandle & session)
-    {
-        Release();
-        Grab(session);
-    }
-
-    bool Contains(const SessionHandle & session) const
-    {
-        return mSession.HasValue() && &mSession.Value().Get() == &session.mSession.Get();
-    }
-
-    bool GrabPairingSession(const SessionHandle & session); // Should be only used inside CASE/PASE pairing.
-    bool Grab(const SessionHandle & session);
-    void Release();
-
-    explicit operator bool() const { return mSession.HasValue(); }
-    Optional<SessionHandle> Get() const
-    {
-        //
-        // We cannot return mSession directly even if Optional<SessionHandle> is internally composed of the same bits,
-        // since they are not actually equivalent type-wise, and SessionHandle does not permit copy-construction.
-        //
-        // So, construct a new Optional<SessionHandle> from the underlying Transport::Session reference.
-        //
-        return mSession.HasValue() ? chip::MakeOptional<SessionHandle>(mSession.Value().Get())
-                                   : chip::Optional<SessionHandle>::Missing();
-    }
-
-    Transport::Session * operator->() const { return &mSession.Value().Get(); }
-
-    // There is not delegate, nothing to do here
-    virtual void DispatchSessionEvent(SessionDelegate::Event event) {}
-
-protected:
-    // Helper for use by the Grab methods.
-    void GrabUnchecked(const SessionHandle & session);
-
-    Optional<ReferenceCountedHandle<Transport::Session>> mSession;
-};
-
-/// @brief Extends SessionHolder to allow propagate SessionDelegate::* events to a given destination
-class SessionHolderWithDelegate : public SessionHolder
-{
-public:
-    SessionHolderWithDelegate(SessionDelegate & delegate) : mDelegate(delegate) {}
-    SessionHolderWithDelegate(const SessionHandle & handle, SessionDelegate & delegate) : SessionHolder(handle), mDelegate(delegate)
-    {}
-    operator bool() const { return SessionHolder::operator bool(); }
-
-    void SessionReleased() override
-    {
-        Release();
-
-        // Note, the session is already cleared during mDelegate.OnSessionReleased
-        mDelegate.OnSessionReleased();
-    }
-
-    void ShiftToSession(const SessionHandle & session) override
-    {
-        if (mDelegate.GetNewSessionHandlingPolicy() == SessionDelegate::NewSessionHandlingPolicy::kShiftToNewSession)
-            SessionHolder::ShiftToSession(session);
-    }
-
-    void DispatchSessionEvent(SessionDelegate::Event event) override { (mDelegate.*event)(); }
-
-private:
-    SessionDelegate & mDelegate;
-};
-
-} // namespace chip
+#include <transport/Session.h>