diff --git a/src/transport/Session.h b/src/transport/Session.h index d9840ec3ee33f1..c18238c1bcacea 100644 --- a/src/transport/Session.h +++ b/src/transport/Session.h @@ -45,6 +45,48 @@ class Session; * 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. + * + * A `SessionHandle` exists to guarantee that the object it points to will not be released while allowing passing + * the handle as a reference, to not incur extra reference-counted Retain/Release calls. + * + * Example code that breaks assumptions (hard to reason about/maintain): + * + * void Process(ReferenceCountedHandle &handle); + * class Foo { + * ReferenceCountedHandle mSession; + * void ResetSession() { mSession = createNewSession(); } + * void DoProcessing() { + * Process(mSession); + * } + * + * static Foo& GetInstance(); + * }; + * + * void Process(ReferenceCountedHandle &handle) { + * Foo::GetInstance()->ResetSession(); // this changes the passed in handle + * // trying to use "&handle" here may point to something else altogether. + * } + * + * Above would be fixed if we would pass in the handles by value, however that adds extra code + * to call Retain/Release every time. We could also by design say that passed in references will + * not change, however historically the codebase is complex enough that this could not be ensured. + * + * The end result is the existence of SessionHandle which is NOT allowed to be held and it serves + * as a marker of "Retain has been called and stays valid". The code above becomes: + * + * void Process(SessionHandle &handle); + * + * .... + * void Foo::DoProcessing() { + * SessionHandle handle(mSession); // retains the session and mSession can be independently changed + * Process(&handle); // reference is now safe to use. + * } + * + * To meet the requirements of "you should not store this", the Handle has additional restrictions + * preventing modification (no assignment or copy constructor) and allows only move. + * NOTE: `move` should likely also not be allowed, however we need to have the ability to + * return such objects from method calls, so it is currently allowed. + * */ class SessionHandle {