JosiahWI commented on code in PR #13804:
URL: https://github.com/apache/trafficserver/pull/13804#discussion_r4223867197
##########
include/iocore/eventsystem/Continuation.h:
##########
@@ -108,83 +149,162 @@ class force_VFPT_to_top
};
/**
- Base class for all state machines to receive notification of
- events.
-
- The Continuation class represents the main abstraction mechanism
- used throughout the IO Core Event System to communicate its users
- the occurrence of an event. A Continuation is a lightweight data
- structure that implements a single method with which the user is
- called back.
-
- Continuations are typically subclassed in order to implement
- event-driven state machines. By including additional state and
- methods, continuations can combine state with control flow, and
- they are generally used to support split-phase, event-driven
- control flow.
-
- Given the multithreaded nature of the Event System, every
- continuation carries a reference to a ProxyMutex object to protect
- its state and ensure atomic operations. This ProxyMutex object
- must be allocated by continuation-derived classes or by clients
- of the IO Core Event System and it is required as a parameter to
- the Continuation's class constructor.
-
+ Base class for event-driven state machines dispatched by the Event
+ System.
+
+ A Continuation pairs the @c handler that @c handleEvent invokes with
+ the @c mutex that serializes those invocations. Derived classes add
+ state and handler methods, and typically replace @c handler as they
+ move between states.
+
+ @note Under multiple inheritance, a derived class MUST list
+ Continuation, directly or through an intermediate base, as its
+ first base class. This works around a known defect: otherwise,
+ installing its handlers may fail to compile with
+ @c -Werror=shift-negative-value.
+
+ @par Lifetime
+ A handler MUST be installed before the Continuation is scheduled or
+ passed to an asynchronous operation. From then on, the Continuation
+ MUST stay alive until no further event can be dispatched to it, i.e.,
+ until each such operation has completed or been cancelled. It may then
+ be destroyed through a @c Continuation pointer, including by
+ @c delete @c this from its own handler, provided nothing, including
+ callers further up the stack, accesses it afterward.
+
+ @par Thread Safety
+ Not thread-safe. While @c mutex is non-null, concurrent accesses must
+ hold it, except where a member documents otherwise; the Event System
+ holds it for the duration of each dispatch.
*/
-
class Continuation : private force_VFPT_to_top
{
public:
/**
- The current continuation handler function.
+ The member function that @c handleEvent invokes.
- The current handler should not be set directly. In order to
- change it, first acquire the Continuation's lock and then use
- the SET_HANDLER macro which takes care of the type casting
- issues.
+ Install a handler with @c SET_HANDLER or @c SET_CONTINUATION_HANDLER
+ rather than by direct assignment; see @c ContinuationHandler for why
+ a @c reinterpret_cast is unsafe. A handler may replace this field
+ while it runs; the new handler receives the next dispatch.
+ @par Thread Safety
+ Not thread-safe.
*/
ContinuationHandler handler = nullptr;
#ifdef DEBUG
+ /**
+ Name of the most recently installed handler, captured by
+ @c SET_HANDLER / @c SET_CONTINUATION_HANDLER for diagnostic use.
+ Present only in DEBUG builds. Initial value is null. Same
+ synchronization rules as @c handler.
+ */
const char *handler_name = nullptr;
#endif
/**
- The Continuation's lock.
-
- A reference counted pointer to the Continuation's lock. This
- lock is initialized in the constructor and should not be set
- directly.
-
- TODO: make this private.
-
+ The mutex held while events are dispatched to this Continuation, or
+ null.
+
+ The Continuation shares ownership of the referenced @c ProxyMutex.
+ If the field is null when an event is scheduled, the scheduler may
+ choose a mutex to hold for that dispatch and may store it here, or
+ may dispatch without holding any mutex. Leave the field null only if
+ the Continuation's state needs no serialization and every
+ asynchronous operation it is passed to accepts a null mutex.
+
+ Reassigning the field while an event or asynchronous operation
+ targeting this Continuation is pending is undefined behavior.
+
+ @par Thread Safety
+ Not thread-safe. Holding the referenced mutex does not protect the
+ field itself, and scheduling this Continuation may write it.
*/
Ptr<ProxyMutex> mutex;
+ /**
+ Returns a raw pointer to the @c ProxyMutex protecting this
+ Continuation, without changing the reference count.
+
+ @return The @c ProxyMutex currently held in @c this->mutex, or
+ nullptr if the field is null. The pointer is valid only
+ while @c this->mutex retains a reference to it; to keep
+ the mutex alive past the Continuation's destruction or a
+ reassignment of @c this->mutex, the caller MUST take its
+ own @c Ptr<ProxyMutex> rather than store the raw pointer.
+
+ @par Thread Safety
+ Caller-synchronized. Callers MUST order this read against any
+ concurrent writers via an external happens-before edge.
+ */
ProxyMutex *
getMutex() const
{
return mutex.get();
}
/**
- Link to other continuations.
-
- A doubly-linked element to allow Lists of Continuations to be
- assembled.
-
+ Doubly-linked list hook used to enqueue this Continuation in
+ intrusive lists whose list-traits class is the nested type
+ @c Continuation::Link_link (declared by the @c LINK macro). Both
+ @c next and @c prev are null-initialized by @c Link<Continuation>'s
+ own default constructor, leaving the hook in the unlinked state.
+
+ @par Thread Safety
+ Plain links. The owner of the list (the Processor or subsystem
+ that holds the queue) is responsible for synchronizing
+ insertion, removal, and traversal. Because there is a single
+ @c next / @c prev pair, the Continuation MUST belong to at most
+ one such list at a time.
*/
LINK(Continuation, link);
/**
- Contains values for debug_override and future flags that
- needs to be thread local while this continuation is running
+ Control flags that may be installed as the current thread's control
+ flags before an event is dispatched to this Continuation.
+
+ Initialized from the constructing thread's current control flags.
+ Scheduling an event for this Continuation may overwrite this field
+ with the scheduling thread's current control flags.
+
+ @par Thread Safety
+ Not thread-safe. Scheduling and some dispatch paths access this
+ field without holding @c mutex, so holding @c mutex is not enough
+ to avoid a data race.
*/
ContFlags control_flags;
+ /**
+ The EThread on which events for this Continuation should preferably
+ be dispatched, or null for no preference.
+
+ The preference is advisory: a Processor may dispatch on another
+ thread, for example when the preferred thread does not service the
+ requested event type. Scheduling an event for this Continuation may
+ set a null field to the thread chosen for that event.
+
+ A non-null value MUST point to an EThread that is alive whenever
+ this Continuation is scheduled.
+
+ @par Thread Safety
+ Not thread-safe. Scheduling accesses this field without holding
+ @c mutex, so holding @c mutex is not enough to avoid a data race.
+ */
EThread *thread_affinity = nullptr;
+ /**
+ Sets the preferred dispatch thread for this Continuation.
+
+ @param[in] ethread The EThread to bind to. Passing nullptr is treated
+ as "no change" (use @c clearThreadAffinity to clear);
+ the call returns false in that case.
Review Comment:
Excellent question. Added a note.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
