<div dir="ltr"><div dir="ltr">Hi Laurent, thank you for the patch.</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Sun, May 23, 2021 at 9:04 AM Laurent Pinchart <<a href="mailto:laurent.pinchart@ideasonboard.com">laurent.pinchart@ideasonboard.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">The LogCategory instances are constructed on first use as static<br>
variables in accessor functions, following the Meyers singleton pattern.<br>
As a result, their destruction order is not guaranteed. This can cause<br>
issues as the global Logger object, constructed in a similar fashion, is<br>
accessed from the LogCategory destructor and may be destroyed first.<br>
<br>
To fix this, keep the same singleton pattern, but allocate the<br>
LogCategory instances dynamically. As they get registered with the<br>
global Logger instance, we can destroy them in the Logger destructor.<br>
<br>
This only avoids destruction order issues between LogCategory and<br>
Logger, and doesn't address yet the fact that LOG() calls from<br>
destructors of global objects may access an already destroyed Logger.<br>
<br>
Signed-off-by: Laurent Pinchart <<a href="mailto:laurent.pinchart@ideasonboard.com" target="_blank">laurent.pinchart@ideasonboard.com</a>><br>
---<br>
include/libcamera/internal/log.h | 5 ++---<br>
src/libcamera/log.cpp | 32 +++++++++++---------------------<br>
2 files changed, 13 insertions(+), 24 deletions(-)<br>
<br>
diff --git a/include/libcamera/internal/log.h b/include/libcamera/internal/log.h<br>
index b66bf55bc57d..1032bdd93e7c 100644<br>
--- a/include/libcamera/internal/log.h<br>
+++ b/include/libcamera/internal/log.h<br>
@@ -29,7 +29,6 @@ class LogCategory<br>
{<br>
public:<br>
explicit LogCategory(const char *name);<br>
- ~LogCategory();</blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
const char *name() const { return name_; }<br>
LogSeverity severity() const { return severity_; }<br>
@@ -48,8 +47,8 @@ extern const LogCategory &_LOG_CATEGORY(name)();<br>
#define LOG_DEFINE_CATEGORY(name) \<br>
const LogCategory &_LOG_CATEGORY(name)() \<br>
{ \<br>
- static LogCategory category(#name); \<br>
- return category; \<br></blockquote><div><br></div><div>I would like to have a comment that the created category will be deleted on Logger destruction?</div><div> </div><div>With that, this change looks good to me.</div><div><br></div><div>Reviewed-by: Hirokazu Honda <<a href="mailto:hiroh@chormium.org">hiroh@chormium.org</a>></div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+ static LogCategory *category = new LogCategory(#name); \<br>
+ return *category; \<br>
}<br>
<br>
class LogMessage<br>
diff --git a/src/libcamera/log.cpp b/src/libcamera/log.cpp<br>
index dd991647b9bc..74829a56916e 100644<br>
--- a/src/libcamera/log.cpp<br>
+++ b/src/libcamera/log.cpp<br>
@@ -248,6 +248,8 @@ void LogOutput::writeStream(const std::string &str)<br>
class Logger<br>
{<br>
public:<br>
+ ~Logger();<br>
+<br>
static Logger *instance();<br>
<br>
void write(const LogMessage &msg);<br>
@@ -267,7 +269,6 @@ private:<br>
<br>
friend LogCategory;<br>
void registerCategory(LogCategory *category);<br>
- void unregisterCategory(LogCategory *category);<br>
<br>
std::unordered_set<LogCategory *> categories_;<br>
std::list<std::pair<std::string, LogSeverity>> levels_;<br>
@@ -369,6 +370,12 @@ void logSetLevel(const char *category, const char *level)<br>
Logger::instance()->logSetLevel(category, level);<br>
}<br>
<br>
+Logger::~Logger()<br>
+{<br>
+ for (LogCategory *category : categories_)<br>
+ delete category;<br>
+}<br>
+<br>
/**<br>
* \brief Retrieve the logger instance<br>
*<br>
@@ -665,18 +672,6 @@ void Logger::registerCategory(LogCategory *category)</blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
}<br>
}<br>
<br>
-/**<br>
- * \brief Unregister a log category from the logger<br>
- * \param[in] category The log category<br>
- *<br>
- * If the \a category hasn't been registered with the logger this function<br>
- * performs no operation.<br>
- */<br>
-void Logger::unregisterCategory(LogCategory *category)<br>
-{<br>
- categories_.erase(category);<br>
-}<br>
-<br>
/**<br>
* \enum LogSeverity<br>
* Log message severity<br>
@@ -711,11 +706,6 @@ LogCategory::LogCategory(const char *name)<br>
Logger::instance()->registerCategory(this);<br>
}<br>
<br>
-LogCategory::~LogCategory()<br>
-{<br>
- Logger::instance()->unregisterCategory(this);<br>
-}<br>
-<br>
/**<br>
* \fn LogCategory::name()<br>
* \brief Retrieve the log category name<br>
@@ -746,12 +736,12 @@ void LogCategory::setSeverity(LogSeverity severity)<br>
* The default log category is named "default" and is used by the LOG() macro<br>
* when no log category is specified.<br>
*<br>
- * \return A pointer to the default log category<br>
+ * \return A reference to the default log category<br>
*/<br>
const LogCategory &LogCategory::defaultCategory()<br>
{<br>
- static const LogCategory category("default");<br>
- return category;<br>
+ static const LogCategory *category = new LogCategory("default");<br>
+ return *category;<br>
}<br>
<br>
/**<br>
-- <br>
Regards,<br>
<br>
Laurent Pinchart<br>
<br>
</blockquote></div></div>