[PATCH 11/12] libcamera: object: Document and ensure Object deletion constraints

[Milan Zamazal]

Laurent Pinchart <laurent.pinchart at ideasonboard.com> writes:

> Object instances are meant to be deleted from the thread they are bound
> to, and this requirement is documented. There are however exceptions to
> the rule, as threads may be stopped and restarted, with objects bound to
> them not being deleted and recreated for every stop/restart cycle. Bound
> objects may therefore need to be deleted after the thread has stopped,
> making it impossible to use Object::deleteLater().
>
> Document the lifetime requirements more precisely, and enforce them with
> an assertion.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

Reviewed-by: Milan Zamazal <mzamazal at redhat.com>

> ---
>  src/libcamera/base/object.cpp | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/src/libcamera/base/object.cpp b/src/libcamera/base/object.cpp
> index 1fce5a2af9af..8af0337f5448 100644
> --- a/src/libcamera/base/object.cpp
> +++ b/src/libcamera/base/object.cpp
> @@ -40,8 +40,9 @@ LOG_DEFINE_CATEGORY(Object)
>   * Object class.
>   *
>   * Deleting an object from a thread other than the one the object is bound to is
> - * unsafe, unless the caller ensures that the object isn't processing any
> - * message concurrently.
> + * unsafe, unless the caller ensures that the object's thread is stopped and no
> + * parent or child of the object gets deleted concurrently. See
> + * Object::~Object() for more information.
>   *
>   * Object slots connected to signals will also run in the context of the
>   * object's thread, regardless of whether the signal is emitted in the same or
> @@ -84,9 +85,20 @@ Object::Object(Object *parent)
>   * Object instances shall be destroyed from the thread they are bound to,
>   * otherwise undefined behaviour may occur. If deletion of an Object needs to
>   * be scheduled from a different thread, deleteLater() shall be used.
> + *
> + * As an exception to this rule, Object instances may be deleted from a
> + * different thread if the thread the instance is bound to is stopped through
> + * the whole duration of the object's destruction, *and* the parent and children
> + * of the object do not get deleted concurrently. The caller is responsible for
> + * fulfilling those requirements.
> + *
> + * In all cases Object instances shall be deleted before the Thread they are
> + * bound to.
>   */
>  Object::~Object()
>  {
> +	ASSERT(Thread::current() == thread_ || !thread_->isRunning());
> +
>  	/*
>  	 * Move signals to a private list to avoid concurrent iteration and
>  	 * deletion of items from Signal::disconnect().