From patchwork Mon Feb 17 18:55:05 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= X-Patchwork-Id: 22807 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id C61EDC3200 for ; Mon, 17 Feb 2025 18:55:11 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 1E38A68695; Mon, 17 Feb 2025 19:55:11 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=protonmail.com header.i=@protonmail.com header.b="YfcyLUDG"; dkim-atps=neutral Received: from mail-4322.protonmail.ch (mail-4322.protonmail.ch [185.70.43.22]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 66A386868F for ; Mon, 17 Feb 2025 19:55:09 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com; s=protonmail3; t=1739818508; x=1740077708; bh=pqomrIDtjkGyEkLgEq9/KVdSaTtPwV82mzxoAhFP/us=; h=Date:To:From:Cc:Subject:Message-ID:In-Reply-To:References: Feedback-ID:From:To:Cc:Date:Subject:Reply-To:Feedback-ID: Message-ID:BIMI-Selector:List-Unsubscribe:List-Unsubscribe-Post; b=YfcyLUDGSYwkL560bSoRx6XVGJD2uxSaVZWu8eD4IKX/VQ7Gn5FxUeF/q4MdNklfv EUthgkAFrzDRZRBMuxSYeQWsyabXMJ5aSzXN3xXOQqQVxCvtC4onaK+NqRpKK3Jgm5 AoX9KbUZwTSlnoiMuP2oRQhxZ0mBzuM/ixFK9gjqUk1ERlK+gZN2dM2QeDGWGLsf9z 3lmn0fuVBxhMOwrY2b1fdPBYXkCTQft/S6ZJGm7vQvhcdL7gzhqvsAN57lGJiZyobL roaQzivh03Cs6VZsO9eScwV1AufJxd1vqZPdvtCaeuYlxj+LejNoulAQ/W6m64y9G1 5f/xSpBjTMBgg== Date: Mon, 17 Feb 2025 18:55:05 +0000 To: libcamera-devel@lists.libcamera.org From: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= Cc: Jacopo Mondi Subject: [PATCH v3 7/8] libcamera: base: log: Protect log categories with lock Message-ID: <20250217185433.306833-8-pobrn@protonmail.com> In-Reply-To: <20250217185433.306833-1-pobrn@protonmail.com> References: <20250217185433.306833-1-pobrn@protonmail.com> Feedback-ID: 20568564:user:proton X-Pm-Message-ID: 2856231cfd9a8cdce6b6b60b560d4693ebd08e90 MIME-Version: 1.0 X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Log categories may be added from any thread, so it is important to synchronize access to the `Logger::categories_` list between its two users: category creation (by LogCategory::create(), which calls Logger::findCategory() and Logger::registerCategory()); and log level setting (by Logger::logSetLevel()). The LogCategory::create() function uses a mutex to serialize category creation, but Logger::logSetLevel() can access `Logger::categories_` concurrently without any protection. To fix the issue, move the mutex to the Logger class, and use it to protect all accesses to the categories list. This requires moving all the logic of LogCategory::create() to a new Logger::findOrCreateCategory() function that combines both Logger::findCategory() and Logger::registerCategory() in order to make the two operations exacute atomically. Signed-off-by: Barnabás Pőcze Reviewed-by: Jacopo Mondi Reviewed-by: Laurent Pinchart --- include/libcamera/base/log.h | 1 + src/libcamera/base/log.cpp | 58 +++++++++++++----------------------- 2 files changed, 22 insertions(+), 37 deletions(-) diff --git a/include/libcamera/base/log.h b/include/libcamera/base/log.h index 195426c41..8ae1b51d0 100644 --- a/include/libcamera/base/log.h +++ b/include/libcamera/base/log.h @@ -39,6 +39,7 @@ public: static const LogCategory &defaultCategory(); private: + friend class Logger; explicit LogCategory(std::string_view name); const std::string name_; diff --git a/src/libcamera/base/log.cpp b/src/libcamera/base/log.cpp index 2c233be36..fd6c11716 100644 --- a/src/libcamera/base/log.cpp +++ b/src/libcamera/base/log.cpp @@ -317,12 +317,12 @@ private: static LogSeverity parseLogLevel(std::string_view level); friend LogCategory; - void registerCategory(LogCategory *category); - LogCategory *findCategory(std::string_view name) const; + LogCategory *findOrCreateCategory(std::string_view name); static bool destroyed_; - std::vector categories_; + Mutex mutex_; + std::vector categories_ LIBCAMERA_TSA_GUARDED_BY(mutex_); std::list> levels_; std::atomic> output_; @@ -572,6 +572,8 @@ void Logger::logSetLevel(const char *category, const char *level) if (severity == LogInvalid) return; + MutexLocker locker(mutex_); + for (LogCategory *c : categories_) { if (c->name() == category) { c->setSeverity(severity); @@ -708,37 +710,28 @@ LogSeverity Logger::parseLogLevel(std::string_view level) } /** - * \brief Register a log category with the logger - * \param[in] category The log category - * - * Log categories must have unique names. It is invalid to call this function - * if a log category with the same name already exists. + * \brief Find an existing log category with the given name or create one + * \param[in] name Name of the log category + * \return The pointer to the log category found or created */ -void Logger::registerCategory(LogCategory *category) +LogCategory *Logger::findOrCreateCategory(std::string_view name) { - categories_.push_back(category); + MutexLocker locker(mutex_); - const std::string &name = category->name(); - for (const auto &[pattern, severity] : levels_) { - if (fnmatch(pattern.c_str(), name.c_str(), FNM_NOESCAPE) == 0) - category->setSeverity(severity); + for (LogCategory *category : categories_) { + if (category->name() == name) + return category; } -} -/** - * \brief Find an existing log category with the given name - * \param[in] name Name of the log category - * \return The pointer to the found log category or nullptr if not found - */ -LogCategory *Logger::findCategory(std::string_view name) const -{ - if (auto it = std::find_if(categories_.begin(), categories_.end(), - [name](auto c) { return c->name() == name; }); - it != categories_.end()) { - return *it; + LogCategory *category = categories_.emplace_back(new LogCategory(name)); + const char *name_cstr = category->name().c_str(); + + for (const auto &[pattern, severity] : levels_) { + if (fnmatch(pattern.c_str(), name_cstr, FNM_NOESCAPE) == 0) + category->setSeverity(severity); } - return nullptr; + return category; } /** @@ -776,16 +769,7 @@ LogCategory *Logger::findCategory(std::string_view name) const */ LogCategory *LogCategory::create(std::string_view name) { - static Mutex mutex_; - MutexLocker locker(mutex_); - LogCategory *category = Logger::instance()->findCategory(name); - - if (!category) { - category = new LogCategory(name); - Logger::instance()->registerCategory(category); - } - - return category; + return Logger::instance()->findOrCreateCategory(name); } /**