[PATCH v6 14/18] Documentation: Add Thread safety page

[Laurent Pinchart]

Hi Dan,

Thank you for the patch.

On Wed, Aug 07, 2024 at 03:22:46PM +0100, Daniel Scally wrote:
> Move the section of the Thread support page dealing with thread
> safety to a dedicated .dox file at Documentation/. This is done to
> support the splitting of the Documentation into a public and internal
> version. With a separate page, references can be made to thread
> safety without having to include the Thread class in the doxygen run.
> 
> Signed-off-by: Daniel Scally <dan.scally at ideasonboard.com>
> ---
> Changes in v6:
> 
> 	- Much of the content that dealt with internal implementation was moved
> 	  back to its place against the Thread class. The thread safety section
> 	  is retained in a separate page, with a single reference to the main
> 	  thread support page who's display is conditional on the doxygen
> 	  INTERNAL_DOCS directive.
> 
> Given the scope of the changes,  I dropped the R-b tags the patch had accumulated
> 
> Changes in v5:
> 
> 	- None
> 
> Changes in v4:
> 
> 	- None
> 
> Changes in v3:
> 
> 	- None
> 
> Changes in v2:
> 
> 	- New patch
> 
>  Documentation/Doxyfile.in       |  4 +++-
>  Documentation/thread-safety.dox | 40 +++++++++++++++++++++++++++++++++
>  src/libcamera/base/thread.cpp   | 36 -----------------------------
>  3 files changed, 43 insertions(+), 37 deletions(-)
>  create mode 100644 Documentation/thread-safety.dox
> 
> diff --git a/Documentation/Doxyfile.in b/Documentation/Doxyfile.in
> index 62e63968..6e5a3830 100644
> --- a/Documentation/Doxyfile.in
> +++ b/Documentation/Doxyfile.in
> @@ -23,7 +23,8 @@ CASE_SENSE_NAMES       = YES
>  QUIET                  = YES
>  WARN_AS_ERROR          = @WARN_AS_ERROR@
>  
> -INPUT                  = "@TOP_SRCDIR@/include/libcamera" \
> +INPUT                  = "@TOP_SRCDIR@/Documentation" \
> +                         "@TOP_SRCDIR@/include/libcamera" \
>                           "@TOP_SRCDIR@/src/ipa/ipu3" \
>                           "@TOP_SRCDIR@/src/ipa/libipa" \
>                           "@TOP_SRCDIR@/src/libcamera" \
> @@ -32,6 +33,7 @@ INPUT                  = "@TOP_SRCDIR@/include/libcamera" \
>  
>  FILE_PATTERNS          = *.c \
>                           *.cpp \
> +                         *.dox \
>                           *.h
>  
>  RECURSIVE              = YES
> diff --git a/Documentation/thread-safety.dox b/Documentation/thread-safety.dox
> new file mode 100644
> index 00000000..d1f8bd37
> --- /dev/null
> +++ b/Documentation/thread-safety.dox
> @@ -0,0 +1,40 @@
> +/**
> + * \page thread-safety Reentrancy and Thread-Safety
> + *
> + * Through the documentation, several terms are used to define how classes and
> + * their member functions can be used from multiple threads.
> + *
> + * - A **reentrant** function may be called simultaneously from multiple
> + *   threads if and only if each invocation uses a different instance of the
> + *   class. This is the default for all member functions not explictly marked
> + *   otherwise.
> + *
> + * - \anchor thread-safe A **thread-safe** function may be called
> + *   simultaneously from multiple threads on the same instance of a class. A
> + *   thread-safe function is thus reentrant. Thread-safe functions may also be
> + *   called simultaneously with any other reentrant function of the same class
> + *   on the same instance.
> + *
> + * - \anchor thread-bound A **thread-bound** function may be called only from
> + *   the thread that the class instances lives in. A thread-bound function is
> + *   not thread-safe, and may or may not be reentrant.
> + *
> + * \internal
> + *   For more information on the implementation of thread-bound functions, see
> + *   section \ref thread-objects.
> + * \endinternal

The thread-bound concept is tied to the Thread class. It shouldn't be
used anywhere in the public API. How about hiding it completely from the
public API documentation ?

 *   \internal
 * - \anchor thread-bound A **thread-bound** function may be called only from
 *   the thread that the class instances lives in (see section \ref
 *   thread-objects). For instances of classes that do not derive from the
 *   Object class, this is the thread in which the instance was created. A
 *   thread-bound function is not thread-safe, and may or may not be reentrant.
 *   \endinternal

> + *
> + * Neither reentrancy nor thread-safety, in this context, mean that a function
> + * may be called simultaneously from the same thread, for instance from a
> + * callback invoked by the function. This may deadlock and isn't allowed unless
> + * separately documented.
> + *
> + * A class is defined as reentrant, thread-safe or thread-bound if all its
> + * member functions are reentrant, thread-safe or thread-bound respectively.
> + * Some member functions may additionally be documented as having additional
> + * thread-related attributes.

We would need to hide the mentiones of thread-bound here too.

> + *
> + * Most classes are reentrant but not thread-safe, as making them fully
> + * thread-safe would incur locking costs considered prohibitive for the
> + * expected use cases.
> + */
> diff --git a/src/libcamera/base/thread.cpp b/src/libcamera/base/thread.cpp
> index 72733431..8735670b 100644
> --- a/src/libcamera/base/thread.cpp
> +++ b/src/libcamera/base/thread.cpp
> @@ -64,42 +64,6 @@
>   * receiver's event loop, running in the receiver's thread. This mechanism can
>   * be overridden by selecting a different connection type when calling
>   * Signal::connect().
> - *
> - * \section thread-reentrancy Reentrancy and Thread-Safety
> - *
> - * Through the documentation, several terms are used to define how classes and
> - * their member functions can be used from multiple threads.
> - *
> - * - A **reentrant** function may be called simultaneously from multiple
> - *   threads if and only if each invocation uses a different instance of the
> - *   class. This is the default for all member functions not explictly marked
> - *   otherwise.
> - *
> - * - \anchor thread-safe A **thread-safe** function may be called
> - *   simultaneously from multiple threads on the same instance of a class. A
> - *   thread-safe function is thus reentrant. Thread-safe functions may also be
> - *   called simultaneously with any other reentrant function of the same class
> - *   on the same instance.
> - *
> - * - \anchor thread-bound A **thread-bound** function may be called only from
> - *   the thread that the class instances lives in (see section \ref
> - *   thread-objects). For instances of classes that do not derive from the
> - *   Object class, this is the thread in which the instance was created. A
> - *   thread-bound function is not thread-safe, and may or may not be reentrant.
> - *
> - * Neither reentrancy nor thread-safety, in this context, mean that a function
> - * may be called simultaneously from the same thread, for instance from a
> - * callback invoked by the function. This may deadlock and isn't allowed unless
> - * separately documented.
> - *
> - * A class is defined as reentrant, thread-safe or thread-bound if all its
> - * member functions are reentrant, thread-safe or thread-bound respectively.
> - * Some member functions may additionally be documented as having additional
> - * thread-related attributes.
> - *
> - * Most classes are reentrant but not thread-safe, as making them fully
> - * thread-safe would incur locking costs considered prohibitive for the
> - * expected use cases.
>   */
>  
>  /**

-- 
Regards,

Laurent Pinchart