Patch Detail
Show a patch.
GET /api/patches/13559/?format=api
{ "id": 13559, "url": "https://patchwork.libcamera.org/api/patches/13559/?format=api", "web_url": "https://patchwork.libcamera.org/patch/13559/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/projects/1/?format=api", "name": "libcamera", "link_name": "libcamera", "list_id": "libcamera_core", "list_email": "libcamera-devel@lists.libcamera.org", "web_url": "", "scm_url": "", "webscm_url": "" }, "msgid": "<20210827155841.17511-1-laurent.pinchart@ideasonboard.com>", "date": "2021-08-27T15:58:41", "name": "[libcamera-devel] libcamera: base: log: Don't crash when logging after Logger destruction", "commit_ref": "c0ca2cbdc2dfcca50d7bc71a93629ca8e02b1682", "pull_url": null, "state": "accepted", "archived": false, "hash": "af7d289f52add054c3f9317f714f370da49a1cdd", "submitter": { "id": 2, "url": "https://patchwork.libcamera.org/api/people/2/?format=api", "name": "Laurent Pinchart", "email": "laurent.pinchart@ideasonboard.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/13559/mbox/", "series": [ { "id": 2413, "url": "https://patchwork.libcamera.org/api/series/2413/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=2413", "date": "2021-08-27T15:58:41", "name": "[libcamera-devel] libcamera: base: log: Don't crash when logging after Logger destruction", "version": 1, "mbox": "https://patchwork.libcamera.org/series/2413/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/13559/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/13559/checks/", "tags": {}, "headers": { "Return-Path": "<libcamera-devel-bounces@lists.libcamera.org>", "X-Original-To": "parsemail@patchwork.libcamera.org", "Delivered-To": "parsemail@patchwork.libcamera.org", "Received": [ "from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 49A2FBD87D\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 27 Aug 2021 15:59:00 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9B5AC6891F;\n\tFri, 27 Aug 2021 17:58:59 +0200 (CEST)", "from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2F41060256\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 27 Aug 2021 17:58:58 +0200 (CEST)", "from pendragon.lan (62-78-145-57.bb.dnainternet.fi [62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id AE5F152F;\n\tFri, 27 Aug 2021 17:58:57 +0200 (CEST)" ], "Authentication-Results": "lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"NsdO+PDA\"; dkim-atps=neutral", "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1630079937;\n\tbh=gj3rEzMm0cbSJ2xB36m9ebkMaut5dJi29CxwWnM83ko=;\n\th=From:To:Cc:Subject:Date:From;\n\tb=NsdO+PDAO2IfW7Le38t+RQBv1GVVsnI8g0DQCecDCn+419QkRmvHSjv3RuoYYd4mM\n\tTRs1t5MknIHAvsZluJlUJbdp8naDe9OxO/QhuJb7cpwIxiCDjEuCvE17t609rQLRyW\n\tSpTEDshukQYuRlYPKDHabY5D6uFEEIvYWKaSYt1s=", "From": "Laurent Pinchart <laurent.pinchart@ideasonboard.com>", "To": "libcamera-devel@lists.libcamera.org", "Date": "Fri, 27 Aug 2021 18:58:41 +0300", "Message-Id": "<20210827155841.17511-1-laurent.pinchart@ideasonboard.com>", "X-Mailer": "git-send-email 2.31.1", "MIME-Version": "1.0", "Content-Transfer-Encoding": "8bit", "Subject": "[libcamera-devel] [PATCH] libcamera: base: log: Don't crash when\n\tlogging after Logger destruction", "X-BeenThere": "libcamera-devel@lists.libcamera.org", "X-Mailman-Version": "2.1.29", "Precedence": "list", "List-Id": "<libcamera-devel.lists.libcamera.org>", "List-Unsubscribe": "<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>", "List-Archive": "<https://lists.libcamera.org/pipermail/libcamera-devel/>", "List-Post": "<mailto:libcamera-devel@lists.libcamera.org>", "List-Help": "<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>", "List-Subscribe": "<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>", "Errors-To": "libcamera-devel-bounces@lists.libcamera.org", "Sender": "\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>" }, "content": "libcamera isn't supposed to log messages after the logger is destroyed,\nas the global logger instance is destroyed after the main() function\nreturns, and the camera manager is supposed to have been stopped and\ndestroyed before that.\n\nThis rule is difficult to enforce in the V4L2 compat implementation, as\nthere is no location where we can destroy the camera manager manually\nbefore the logger is destroyed. This results in a use-after-free\ncondition when the camera manager gets stopped during destruction.\n\nFix it by not trying to print log messages when the global logger\ninstance has been destroyed.\n\nThis is a bit of a hack, but hopefully not too bad. There could be race\nconditions when using a CameraManager instance that is destroyed as part\nof the destruction of global variables (like the V4L2 compat layer does,\nit wraps CameraManager in a singleton V4L2CompatManager class, and\ndestroys it when V4L2CompatManager is destroyed) as the CameraManager\nthread will still be running when the logger gets destroyed, but this\ndoesn't cause any regression as we destroy the logger without any\nsafeguard measure today anyway.\n\nThere are other options that could be considered. Forcing destruction of\nthe logger after the camera manager in the V4L2 compat layer is one of\nthem, but turned out to be difficult. For instance care would need to be\ntaken *not* to log any message in the mmap() wrapper if the fd doesn't\nmatch a wrapped camera, as mmap() is called very early in the\ninitialization process, before libcamera and the logger get initialized.\nThe resulting implementation would likely be fairly complex.\n\nAnother option could be to wrap the logger with a shared pointer, and\nkeep a reference to it in CameraManager. That's more intrusive, and it's\nnot clear if it would be worth it.\n\nSigned-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n---\n src/libcamera/base/log.cpp | 18 ++++++++++++++++--\n 1 file changed, 16 insertions(+), 2 deletions(-)", "diff": "diff --git a/src/libcamera/base/log.cpp b/src/libcamera/base/log.cpp\nindex 073b7c34b75e..a3e3f9ea2712 100644\n--- a/src/libcamera/base/log.cpp\n+++ b/src/libcamera/base/log.cpp\n@@ -270,12 +270,16 @@ private:\n \tfriend LogCategory;\n \tvoid registerCategory(LogCategory *category);\n \n+\tstatic bool destroyed_;\n+\n \tstd::unordered_set<LogCategory *> categories_;\n \tstd::list<std::pair<std::string, LogSeverity>> levels_;\n \n \tstd::shared_ptr<LogOutput> output_;\n };\n \n+bool Logger::destroyed_ = false;\n+\n /**\n * \\enum LoggingTarget\n * \\brief Log destination type\n@@ -372,6 +376,8 @@ void logSetLevel(const char *category, const char *level)\n \n Logger::~Logger()\n {\n+\tdestroyed_ = true;\n+\n \tfor (LogCategory *category : categories_)\n \t\tdelete category;\n }\n@@ -387,6 +393,10 @@ Logger::~Logger()\n Logger *Logger::instance()\n {\n \tstatic Logger instance;\n+\n+\tif (destroyed_)\n+\t\treturn nullptr;\n+\n \treturn &instance;\n }\n \n@@ -808,13 +818,17 @@ LogMessage::~LogMessage()\n \tif (severity_ == LogInvalid)\n \t\treturn;\n \n+\tLogger *logger = Logger::instance();\n+\tif (!logger)\n+\t\treturn;\n+\n \tmsgStream_ << std::endl;\n \n \tif (severity_ >= category_.severity())\n-\t\tLogger::instance()->write(*this);\n+\t\tlogger->write(*this);\n \n \tif (severity_ == LogSeverity::LogFatal) {\n-\t\tLogger::instance()->backtrace();\n+\t\tlogger->backtrace();\n \t\tstd::abort();\n \t}\n }\n", "prefixes": [ "libcamera-devel" ] }