[libcamera-devel] [PATCH] libcamera: log: Document LOG() restrictions

[Laurent Pinchart]

Hi Kieran,

On Fri, May 21, 2021 at 11:16:40AM +0100, Kieran Bingham wrote:
> On 21/05/2021 10:22, Laurent Pinchart wrote:
> > The LOG() macro uses the global Logger and LogCategory objects
> > internally. This can result in crashes or other undefined behaviour when
> > logging from destructors of global objects, as the Logger and/or the
> > LogCategory instances may have been destroyed.
> > 
> > This isn't ideal and should eventually be fixed. For the time being,
> > document the restriction, and add a todo list item to fix the problem.
> 
> Is there a way to catch this in code? Presumably it's something to do
> with the global logging object being destructed, and then an action
> occuring on that object?

Correct.

> Could we have something like the canary I proposed on the requests, so
> that if any action on the log happens after it's destructed it fires a
> more meaningful assert rather than a less parsable segfault/random failure?

That should be possible, but I'd rather focus on addressing the todo
item below, to allow logging messages everywhere (expect probably from
the destructor of the Logger itself :-)).

> > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > ---
> >  src/libcamera/log.cpp | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/src/libcamera/log.cpp b/src/libcamera/log.cpp
> > index 94175ab34535..985952249289 100644
> > --- a/src/libcamera/log.cpp
> > +++ b/src/libcamera/log.cpp
> > @@ -980,6 +980,12 @@ LogMessage _log(const LogCategory *category, LogSeverity severity,
> >   *
> >   * If the severity is set to Fatal, execution is aborted and the program
> >   * terminates immediately after printing the message.
> > + *
> > + * \warning Logging from the destructor of a global object, either directly or
> > + * indirectly, results in undefined behaviour.
> > + *
> > + * \todo Allow logging from destructors of global objects to the largest
> > + * possible extent
> 
> I guess there's no way to say "Please delete me last"?
> 
> Are there any tricks that can be played by trying to ensure the log is
> the first thing constructed?

I may start by looking at LogCategory first, to allocate them
dynamically and delete them when the Logger is deleted. When it comes to
the Logger itself, I'm not sure yet.

> Anyway, documenting the current limitation is a good thing
> 
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> 
> >   */
> >  
> >  /**

-- 
Regards,

Laurent Pinchart