[libcamera-devel] [PATCH 11/19] libcamera: Document thread-safety attributes of core classes
Jacopo Mondi
jacopo at jmondi.org
Mon Jan 20 15:58:26 CET 2020
Hi Laurent,
On Mon, Jan 20, 2020 at 02:24:29AM +0200, Laurent Pinchart wrote:
> Define the thread-safety attributes of the classes and methods that are
> either thread-safe or thread-bound. The CameraManager, Camera and
> PipelineHandler will be addressed separately.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
> src/libcamera/device_enumerator.cpp | 2 ++
> src/libcamera/event_notifier.cpp | 2 ++
> src/libcamera/ipc_unixsocket.cpp | 2 ++
> src/libcamera/object.cpp | 10 ++++++++--
> src/libcamera/thread.cpp | 6 +++++-
> src/libcamera/timer.cpp | 20 ++++++++++++--------
> src/libcamera/v4l2_videodevice.cpp | 2 ++
> 7 files changed, 33 insertions(+), 11 deletions(-)
>
> diff --git a/src/libcamera/device_enumerator.cpp b/src/libcamera/device_enumerator.cpp
> index a8b5c90f5a5d..64cdc132308a 100644
> --- a/src/libcamera/device_enumerator.cpp
> +++ b/src/libcamera/device_enumerator.cpp
> @@ -190,6 +190,8 @@ DeviceEnumerator::~DeviceEnumerator()
> * with a warning message logged, without returning an error. Only errors that
> * prevent enumeration altogether shall be fatal.
> *
> + * \context This function is \threadbound.
> + *
I'm trying to grasp why enumeration is thread bound
> * \return 0 on success or a negative error code otherwise
> */
>
> diff --git a/src/libcamera/event_notifier.cpp b/src/libcamera/event_notifier.cpp
> index 4326b0b413e2..a9be686f79ae 100644
> --- a/src/libcamera/event_notifier.cpp
> +++ b/src/libcamera/event_notifier.cpp
> @@ -99,6 +99,8 @@ EventNotifier::~EventNotifier()
> *
> * This function enables or disables the notifier. A disabled notifier ignores
> * events and does not emit the \ref activated signal.
> + *
> + * \context This function is \threadbound.
> */
> void EventNotifier::setEnabled(bool enable)
> {
> diff --git a/src/libcamera/ipc_unixsocket.cpp b/src/libcamera/ipc_unixsocket.cpp
> index eb1a50239188..6e5cab894a93 100644
> --- a/src/libcamera/ipc_unixsocket.cpp
> +++ b/src/libcamera/ipc_unixsocket.cpp
> @@ -62,6 +62,8 @@ LOG_DEFINE_CATEGORY(IPCUnixSocket)
> * communication method. The remote side then instantiates a socket, and binds
> * it to the other side by passing the file descriptor to bind(). At that point
> * the channel is operation and communication is bidirectional and symmmetrical.
> + *
> + * \context This class is \threadbound.
I'm wondering if a "why" statement would be beneficial. I had to go
and look to find out the class instantiates an EventNotifier bound to
the current thread.
> */
>
> IPCUnixSocket::IPCUnixSocket()
> diff --git a/src/libcamera/object.cpp b/src/libcamera/object.cpp
> index f2a8be172547..99c3bf9a709b 100644
> --- a/src/libcamera/object.cpp
> +++ b/src/libcamera/object.cpp
> @@ -110,6 +110,8 @@ Object::~Object()
> * Messages are delivered through the thread's event loop. If the thread is not
> * running its event loop the message will not be delivered until the event
> * loop gets started.
> + *
> + * \context This function is \threadsafe.
> */
> void Object::postMessage(std::unique_ptr<Message> msg)
> {
> @@ -162,6 +164,8 @@ void Object::message(Message *msg)
> * are passed untouched. The caller shall ensure that any pointer argument
> * remains valid until the method is invoked.
> *
> + * \context This function is \threadsafe.
> + *
they will be since next patch :)
> * \return For connection types ConnectionTypeDirect and
> * ConnectionTypeBlocking, return the return value of the invoked method. For
> * connection type ConnectionTypeQueued, return a default-constructed R value.
> @@ -170,6 +174,7 @@ void Object::message(Message *msg)
> /**
> * \fn Object::thread()
> * \brief Retrieve the thread the object is bound to
> + * \context This function is \threadsafe.
> * \return The thread the object is bound to
> */
>
> @@ -178,8 +183,7 @@ void Object::message(Message *msg)
> * \param[in] thread The target thread
> *
> * This method moves the object and all its children from the current thread to
> - * the new \a thread. It shall be called from the thread in which the object
> - * currently lives, otherwise the behaviour is undefined.
> + * the new \a thread.
> *
> * Before the object is moved, a Message::ThreadMoveMessage message is sent to
> * it. The message() method can be reimplement in derived classes to be notified
> @@ -187,6 +191,8 @@ void Object::message(Message *msg)
> *
> * Moving an object that has a parent is not allowed, and causes undefined
> * behaviour.
> + *
> + * \context This function is thread-bound.
> */
> void Object::moveToThread(Thread *thread)
> {
> diff --git a/src/libcamera/thread.cpp b/src/libcamera/thread.cpp
> index c1698d469a6c..0cf6ff691315 100644
> --- a/src/libcamera/thread.cpp
> +++ b/src/libcamera/thread.cpp
> @@ -40,7 +40,9 @@
> *
> * - \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 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.
Is this intentionally not squashed in the previous patch ?
> *
> * - \anchor thread-bound A **thread-bound** function may be called only from
> * the thread that the class instances lives in (see section \ref
> @@ -220,6 +222,8 @@ ThreadData *ThreadData::current()
> * called. A custom event dispatcher may be installed with
> * setEventDispatcher(), otherwise a poll-based event dispatcher is used. This
> * behaviour can be overriden by overloading the run() method.
> + *
> + * \context This class is \threadsafe.
> */
>
> /**
> diff --git a/src/libcamera/timer.cpp b/src/libcamera/timer.cpp
> index 4c68883204e8..24da51524efb 100644
> --- a/src/libcamera/timer.cpp
> +++ b/src/libcamera/timer.cpp
> @@ -67,16 +67,18 @@ Timer::~Timer()
> * \brief Start or restart the timer with a timeout of \a msec
> * \param[in] msec The timer duration in milliseconds
> *
> - * This method shall be called from the thread the timer is associated with. If
> - * the timer is already running it will be stopped and restarted.
> + * If the timer is already running it will be stopped and restarted.
> + *
> + * \context This function is \threadbound.
> */
>
> /**
> * \brief Start or restart the timer with a timeout of \a duration
> * \param[in] duration The timer duration in milliseconds
> *
> - * This method shall be called from the thread the timer is associated with. If
> - * the timer is already running it will be stopped and restarted.
> + * If the timer is already running it will be stopped and restarted.
> + *
> + * \context This function is \threadbound.
> */
> void Timer::start(std::chrono::milliseconds duration)
> {
> @@ -87,8 +89,9 @@ void Timer::start(std::chrono::milliseconds duration)
> * \brief Start or restart the timer with a \a deadline
> * \param[in] deadline The timer deadline
> *
> - * This method shall be called from the thread the timer is associated with. If
> - * the timer is already running it will be stopped and restarted.
> + * If the timer is already running it will be stopped and restarted.
> + *
> + * \context This function is \threadbound.
> */
> void Timer::start(std::chrono::steady_clock::time_point deadline)
> {
> @@ -115,8 +118,9 @@ void Timer::start(std::chrono::steady_clock::time_point deadline)
> * After this function returns the timer is guaranteed not to emit the
> * \ref timeout signal.
> *
> - * This method shall be called from the thread the timer is associated with. If
> - * the timer is not running this function performs no operation.
> + * If the timer is not running this function performs no operation.
> + *
> + * \context This function is \threadbound.
> */
> void Timer::stop()
> {
> diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
> index 51be1dcd7fff..cb166c795abd 100644
> --- a/src/libcamera/v4l2_videodevice.cpp
> +++ b/src/libcamera/v4l2_videodevice.cpp
> @@ -404,6 +404,8 @@ const std::string V4L2DeviceFormat::toString() const
> *
> * Upon destruction any device left open will be closed, and any resources
> * released.
> + *
> + * \context This class is \threadbound.
> */
>
> /**
> --
> Regards,
>
> Laurent Pinchart
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20200120/09b11895/attachment.sig>
More information about the libcamera-devel
mailing list