JosiahWI commented on code in PR #13882:
URL: https://github.com/apache/trafficserver/pull/13882#discussion_r4449782948
##########
include/iocore/eventsystem/Thread.h:
##########
@@ -67,55 +34,147 @@
class ProxyMutex;
+/**
+ Size in bytes, including the terminating NUL, of a thread name buffer.
+
+ Use it to size buffers that build or read back the names of ATS
+ threads. A thread name holds at most @c MAX_THREAD_NAME_LENGTH - 1
+ bytes; longer names are truncated when the thread is started, without
+ any error being reported.
+*/
constexpr int MAX_THREAD_NAME_LENGTH = 16;
-/// The signature of a function to be called by a thread.
+/**
+ Optional entry point for the OS thread spawned by @c Thread::start.
+
+ When non-empty, the spawned thread invokes the callable exactly once,
+ in place of @c Thread::execute, and exits when it returns.
+
+ @note @c Thread::start keeps its own copy of the callable. Anything the
+ callable captures by reference or pointer MUST stay valid until
+ the callable returns.
+
+ @pre The callable MUST NOT exit via an exception.
+
+ @par Thread Safety
+ The callable runs on the spawned thread, concurrently with the caller
+ of @c Thread::start. State it shares with other threads needs its own
+ synchronization.
+*/
using ThreadFunction = std::function<void()>;
/**
- Base class for the threads in the Event System. Thread is the base
- class for all the thread classes in the Event System. Objects of the
- Thread class represent spawned or running threads and provide minimal
- information for its derived classes. Thread objects have a reference
- to a ProxyMutex, that is used for atomic operations internally, and
- an ink_thread member that is used to identify the thread in the system.
-
- You should not create an object of the Thread class, they are typically
- instantiated after some thread startup mechanism exposed by a processor,
- but even then you would probably deal with processor functions and
- not the Thread object itself.
+ Base class for an object that represents one OS thread and holds its
+ per-thread state.
+ A @c Thread represents the OS thread on which it is registered in
+ @c this_thread_ptr.
+
+ @par Lifetime
+ A @c Thread MUST NOT be destroyed while a different OS thread may
+ still use it, for example through @c this_thread_ptr or a running
+ @c execute.
*/
class Thread
{
public:
/**
- System-wide thread identifier. The thread identifier is represented
- by the platform independent type ink_thread and it is the system-wide
- value assigned to each thread. It is exposed as a convenience for
- processors and you should not modify it directly.
-
+ Identifier of the OS thread spawned by @c start.
+
+ Zero until @c start is called, and remains zero for a @c Thread that
+ is registered via @c set_specific without being started. The value
+ is never reset: once the OS thread has been joined, it no longer
+ identifies a live thread and MAY be reused by the platform.
+
+ @par Thread Safety
+ Written by @c start on its calling thread before @c start returns.
+ Safe to read from that thread after @c start returns, and from any
+ thread that synchronizes with it afterward. The spawned thread
+ itself MUST NOT read @c tid without such synchronization; it should
+ obtain its own identifier from the platform instead.
*/
// NOLINTNEXTLINE(modernize-use-nullptr)
ink_thread tid = 0;
/**
- Thread lock to ensure atomic operations. The thread lock available
- to derived classes to ensure atomic operations and protect critical
- regions. Do not modify this member directly.
+ Mutex that the @c Thread holds on its own behalf.
+
+ Non-null and unchanged from construction until destruction; do not
+ assign to it. Once construction completes, the @c Thread holds this
+ mutex until destruction, so a lock taken on the OS thread this
Review Comment:
This comment does not contradict the constructor postcondition. The mutex is
unlocked on return of the base `Thread` constructor and it is fair (and useful)
to require subclasses to hold the mutex until destruction. I'll look at
rewording the comment to make the intent more clear.
> so callers do not rely on exclusion that Thread itself does not provide
To elaborate a bit, that would actually be a bad thing for the API, because
it would mean that if you have a `Thread` pointer which is effectively a
pointer to an `EThread`, you cannot assume the mutex is locked. Basically, if
you remove constraints on `Thread`, then the entire intended benefit of
`EThread` holding the mutex the whole time is immediately lost as soon as you
access it through a `Thread` pointer.
--
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]
