[{"id":17071,"web_url":"https://patchwork.libcamera.org/comment/17071/","msgid":"","date":"2021-05-21T09:26:55","subject":"Re: [libcamera-devel] [PATCH] libcamera: log: Document LOG()\n\trestrictions","submitter":{"id":63,"url":"https://patchwork.libcamera.org/api/people/63/","name":"Hirokazu Honda","email":"hiroh@chromium.org"},"content":"Hi Laurent, thank you for the patch.\n\nOn Fri, May 21, 2021 at 6:22 PM Laurent Pinchart <\nlaurent.pinchart@ideasonboard.com> wrote:\n\n> The LOG() macro uses the global Logger and LogCategory objects\n> internally. This can result in crashes or other undefined behaviour when\n> logging from destructors of global objects, as the Logger and/or the\n> LogCategory instances may have been destroyed.\n>\n> This isn't ideal and should eventually be fixed. For the time being,\n> document the restriction, and add a todo list item to fix the problem.\n>\n> Signed-off-by: Laurent Pinchart \n>\n\nReviewed-by: Hirokazu Honda \n\n\n> ---\n> src/libcamera/log.cpp | 6 ++++++\n> 1 file changed, 6 insertions(+)\n>\n> diff --git a/src/libcamera/log.cpp b/src/libcamera/log.cpp\n> index 94175ab34535..985952249289 100644\n> --- a/src/libcamera/log.cpp\n> +++ b/src/libcamera/log.cpp\n> @@ -980,6 +980,12 @@ LogMessage _log(const LogCategory *category,\n> LogSeverity severity,\n> *\n> * If the severity is set to Fatal, execution is aborted and the program\n> * terminates immediately after printing the message.\n> + *\n> + * \\warning Logging from the destructor of a global object, either\n> directly or\n> + * indirectly, results in undefined behaviour.\n> + *\n> + * \\todo Allow logging from destructors of global objects to the largest\n> + * possible extent\n> */\n>\n> /**\n> --\n> Regards,\n>\n> Laurent Pinchart\n>\n>","headers":{"Return-Path":"","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 E405AC31FB\n\tfor ;\n\tFri, 21 May 2021 09:27:08 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 593576891B;\n\tFri, 21 May 2021 11:27:08 +0200 (CEST)","from mail-ed1-x534.google.com (mail-ed1-x534.google.com\n\t[IPv6:2a00:1450:4864:20::534])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1A87368918\n\tfor ;\n\tFri, 21 May 2021 11:27:07 +0200 (CEST)","by mail-ed1-x534.google.com with SMTP id s6so22526500edu.10\n\tfor ;\n\tFri, 21 May 2021 02:27:07 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=chromium.org header.i=@chromium.org\n\theader.b=\"ImpQRYEo\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org;\n\ts=google; \n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=XClBqPixfOehXDMsPbIyfHPYx8NIZjyKaMMyWa7PGY0=;\n\tb=ImpQRYEoc5qYHH+EQdhZ5tv0u3fMmkGZreDvCDLzqakoWunNTNv9YSUbjJ4YzTZuqE\n\tbXpEdtq8/oUHc+ls9xd7562UGWSDC4yqg6uVs/Fw0xQUC4shfFyRqFSHpPN2aHTtkUoq\n\tFdc1TsdE5P+6bV6p+GDAfIrA5v6Vrt32lHFCw=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=XClBqPixfOehXDMsPbIyfHPYx8NIZjyKaMMyWa7PGY0=;\n\tb=dS4bgJ2ibtRZ44bDHtGlYTrwvWkvbhB7mpYWYHBiYEw03GskExSeL2bH1Cn0poQ4Bw\n\tIYwGU/Mp2qokyaa21v5tQDsx0h38T3C1xEU/Ac83mPbUwgt6/91jAWvbSaE/gQBFfepC\n\tUebhW8fuQ+3VpDAqGXpsURnFO0IHQ6JFUvkZHeSNpM9K2lXtmqhneq+zY28UrnPdARKZ\n\tL6T+hK57puq6o674RU5TQU2LG4HvT+q0kBqCaq7Ffbjeqacx7NQI0dWiEpsZpL8J6mle\n\telq6sGsiSwR+/eRmwdQzZ+sAbjvtOvWTigPufXNi05xHxu2/A0DNYlBt89dnKrNl+HuN\n\tplPQ==","X-Gm-Message-State":"AOAM532kwmAXLh1I84vavCsXXggNtrqu2WFeF5z5XtJST6A6oaydKsp3\n\tsGkfeVtZpRZ08z8i/0ItpfRgrTWnr1tcjaSAIuiKHg==","X-Google-Smtp-Source":"ABdhPJz4L0cdfxdVKOeP4F4ygKJDqQw80OhvwESR3H7IB9pGflb0Slwzf1ozqooHtshW+H8PnhpaZ3HCc7EBaa1rHxw=","X-Received":"by 2002:a05:6402:cb8:: with SMTP id\n\tcn24mr9980528edb.325.1621589226756; \n\tFri, 21 May 2021 02:27:06 -0700 (PDT)","MIME-Version":"1.0","References":"<20210521092214.1160-1-laurent.pinchart@ideasonboard.com>","In-Reply-To":"<20210521092214.1160-1-laurent.pinchart@ideasonboard.com>","From":"Hirokazu Honda ","Date":"Fri, 21 May 2021 18:26:55 +0900","Message-ID":"","To":"Laurent Pinchart ","Content-Type":"multipart/alternative; boundary=\"000000000000d5331805c2d3a9cd\"","Subject":"Re: [libcamera-devel] [PATCH] libcamera: log: Document LOG()\n\trestrictions","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":17078,"web_url":"https://patchwork.libcamera.org/comment/17078/","msgid":"<20210521100037.GI902042@pyrite.rasen.tech>","date":"2021-05-21T10:00:37","subject":"Re: [libcamera-devel] [PATCH] libcamera: log: Document LOG()\n\trestrictions","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"On Fri, May 21, 2021 at 12:22:14PM +0300, Laurent Pinchart wrote:\n> The LOG() macro uses the global Logger and LogCategory objects\n> internally. This can result in crashes or other undefined behaviour when\n> logging from destructors of global objects, as the Logger and/or the\n> LogCategory instances may have been destroyed.\n> \n> This isn't ideal and should eventually be fixed. For the time being,\n> document the restriction, and add a todo list item to fix the problem.\n> \n> Signed-off-by: Laurent Pinchart \n\nReviewed-by: Paul Elder \n\n> ---\n> src/libcamera/log.cpp | 6 ++++++\n> 1 file changed, 6 insertions(+)\n> \n> diff --git a/src/libcamera/log.cpp b/src/libcamera/log.cpp\n> index 94175ab34535..985952249289 100644\n> --- a/src/libcamera/log.cpp\n> +++ b/src/libcamera/log.cpp\n> @@ -980,6 +980,12 @@ LogMessage _log(const LogCategory *category, LogSeverity severity,\n> *\n> * If the severity is set to Fatal, execution is aborted and the program\n> * terminates immediately after printing the message.\n> + *\n> + * \\warning Logging from the destructor of a global object, either directly or\n> + * indirectly, results in undefined behaviour.\n> + *\n> + * \\todo Allow logging from destructors of global objects to the largest\n> + * possible extent\n> */\n> \n> /**\n> -- \n> Regards,\n> \n> Laurent Pinchart\n>","headers":{"Return-Path":"","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 E2819C31FF\n\tfor ;\n\tFri, 21 May 2021 10:00:45 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9C32F6891D;\n\tFri, 21 May 2021 12:00:45 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2E72568911\n\tfor ;\n\tFri, 21 May 2021 12:00:44 +0200 (CEST)","from pyrite.rasen.tech (unknown\n\t[IPv6:2400:4051:61:600:2c71:1b79:d06d:5032])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id D4CFA8D8;\n\tFri, 21 May 2021 12:00:42 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"Y55Praq8\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1621591243;\n\tbh=fddkaEAjfW+vFbj9YFTwgqrQfaIOk/jSS7WSkh769dg=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Y55Praq8A+HEZXzPSj2U1j6Ub26X5ZibQaAEANt02lDx+1DHhH/LayFI30jv2MmhM\n\tGW8jvR+CFCPjZNVLDtxwAXkNM4sIp4QAoATKXcGk4wZE9P9euvCDhY93qafm2T6hW4\n\t8p1FzYw66OkwqCcgCx056yNUFKC4SF54lLOzk3N4=","Date":"Fri, 21 May 2021 19:00:37 +0900","From":"paul.elder@ideasonboard.com","To":"Laurent Pinchart ","Message-ID":"<20210521100037.GI902042@pyrite.rasen.tech>","References":"<20210521092214.1160-1-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20210521092214.1160-1-laurent.pinchart@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH] libcamera: log: Document LOG()\n\trestrictions","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":17079,"web_url":"https://patchwork.libcamera.org/comment/17079/","msgid":"","date":"2021-05-21T10:16:40","subject":"Re: [libcamera-devel] [PATCH] libcamera: log: Document LOG()\n\trestrictions","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Laurent,\n\nOn 21/05/2021 10:22, Laurent Pinchart wrote:\n> The LOG() macro uses the global Logger and LogCategory objects\n> internally. This can result in crashes or other undefined behaviour when\n> logging from destructors of global objects, as the Logger and/or the\n> LogCategory instances may have been destroyed.\n> \n> This isn't ideal and should eventually be fixed. For the time being,\n> document the restriction, and add a todo list item to fix the problem.\n\nIs there a way to catch this in code? Presumably it's something to do\nwith the global logging object being destructed, and then an action\noccuring on that object?\n\nCould we have something like the canary I proposed on the requests, so\nthat if any action on the log happens after it's destructed it fires a\nmore meaningful assert rather than a less parsable segfault/random failure?\n\n\n> Signed-off-by: Laurent Pinchart \n> ---\n> src/libcamera/log.cpp | 6 ++++++\n> 1 file changed, 6 insertions(+)\n> \n> diff --git a/src/libcamera/log.cpp b/src/libcamera/log.cpp\n> index 94175ab34535..985952249289 100644\n> --- a/src/libcamera/log.cpp\n> +++ b/src/libcamera/log.cpp\n> @@ -980,6 +980,12 @@ LogMessage _log(const LogCategory *category, LogSeverity severity,\n> *\n> * If the severity is set to Fatal, execution is aborted and the program\n> * terminates immediately after printing the message.\n> + *\n> + * \\warning Logging from the destructor of a global object, either directly or\n> + * indirectly, results in undefined behaviour.\n> + *\n> + * \\todo Allow logging from destructors of global objects to the largest\n> + * possible extent\n\nI guess there's no way to say \"Please delete me last\"?\n\nAre there any tricks that can be played by trying to ensure the log is\nthe first thing constructed?\n\nAnyway, documenting the current limitation is a good thing\n\nReviewed-by: Kieran Bingham \n\n> */\n> \n> /**\n>","headers":{"Return-Path":"","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 D3025C3200\n\tfor ;\n\tFri, 21 May 2021 10:16:45 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 32A156891A;\n\tFri, 21 May 2021 12:16:45 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C870D68911\n\tfor ;\n\tFri, 21 May 2021 12:16:43 +0200 (CEST)","from [192.168.0.20]\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 0875C8D8;\n\tFri, 21 May 2021 12:16:42 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"Qpw4PNaS\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1621592203;\n\tbh=5ZjKgHqR7x3wImhHOR/QVcFq0i/nMFVsbZP6fnaCY/I=;\n\th=Reply-To:To:References:From:Subject:Date:In-Reply-To:From;\n\tb=Qpw4PNaS4lfq3mY1bYm6S50tQg7vw92911oQVyIUCdWJI0neKgJQpxNFu8g5uwiAm\n\t2VBO/Ii8w0Rb3HlAZZmNsHmEGBFsnIjzirnlPoYdhlDIdyvIF4gOR6izOvu6+iPKWV\n\tJYqzbOPCkx3Z+pDencSXuktnWfo1T4yM7xVWRzy4=","To":"Laurent Pinchart ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20210521092214.1160-1-laurent.pinchart@ideasonboard.com>","From":"Kieran Bingham ","Organization":"Ideas on Board","Message-ID":"","Date":"Fri, 21 May 2021 11:16:40 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.8.1","MIME-Version":"1.0","In-Reply-To":"<20210521092214.1160-1-laurent.pinchart@ideasonboard.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-GB","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH] libcamera: log: Document LOG()\n\trestrictions","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Reply-To":"kieran.bingham@ideasonboard.com","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":17108,"web_url":"https://patchwork.libcamera.org/comment/17108/","msgid":"","date":"2021-05-21T23:59:42","subject":"Re: [libcamera-devel] [PATCH] libcamera: log: Document LOG()\n\trestrictions","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kieran,\n\nOn Fri, May 21, 2021 at 11:16:40AM +0100, Kieran Bingham wrote:\n> On 21/05/2021 10:22, Laurent Pinchart wrote:\n> > The LOG() macro uses the global Logger and LogCategory objects\n> > internally. This can result in crashes or other undefined behaviour when\n> > logging from destructors of global objects, as the Logger and/or the\n> > LogCategory instances may have been destroyed.\n> > \n> > This isn't ideal and should eventually be fixed. For the time being,\n> > document the restriction, and add a todo list item to fix the problem.\n> \n> Is there a way to catch this in code? Presumably it's something to do\n> with the global logging object being destructed, and then an action\n> occuring on that object?\n\nCorrect.\n\n> Could we have something like the canary I proposed on the requests, so\n> that if any action on the log happens after it's destructed it fires a\n> more meaningful assert rather than a less parsable segfault/random failure?\n\nThat should be possible, but I'd rather focus on addressing the todo\nitem below, to allow logging messages everywhere (expect probably from\nthe destructor of the Logger itself :-)).\n\n> > Signed-off-by: Laurent Pinchart \n> > ---\n> > src/libcamera/log.cpp | 6 ++++++\n> > 1 file changed, 6 insertions(+)\n> > \n> > diff --git a/src/libcamera/log.cpp b/src/libcamera/log.cpp\n> > index 94175ab34535..985952249289 100644\n> > --- a/src/libcamera/log.cpp\n> > +++ b/src/libcamera/log.cpp\n> > @@ -980,6 +980,12 @@ LogMessage _log(const LogCategory *category, LogSeverity severity,\n> > *\n> > * If the severity is set to Fatal, execution is aborted and the program\n> > * terminates immediately after printing the message.\n> > + *\n> > + * \\warning Logging from the destructor of a global object, either directly or\n> > + * indirectly, results in undefined behaviour.\n> > + *\n> > + * \\todo Allow logging from destructors of global objects to the largest\n> > + * possible extent\n> \n> I guess there's no way to say \"Please delete me last\"?\n> \n> Are there any tricks that can be played by trying to ensure the log is\n> the first thing constructed?\n\nI may start by looking at LogCategory first, to allocate them\ndynamically and delete them when the Logger is deleted. When it comes to\nthe Logger itself, I'm not sure yet.\n\n> Anyway, documenting the current limitation is a good thing\n> \n> Reviewed-by: Kieran Bingham \n> \n> > */\n> > \n> > /**","headers":{"Return-Path":"","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 5EF9AC3201\n\tfor ;\n\tFri, 21 May 2021 23:59:49 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 96DC16891E;\n\tSat, 22 May 2021 01:59:48 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9370B602B2\n\tfor ;\n\tSat, 22 May 2021 01:59:46 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id F15C94A8;\n\tSat, 22 May 2021 01:59:45 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"Qs0mu6Yq\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1621641586;\n\tbh=KH3RtZvq99N8Pz4PL8Cb12nt0+TFv31WrcYfdFlfnnc=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Qs0mu6YqwtX7zPN6P9iRanRXyOL7CjmAlcAchoKD58GgSt5qpf9sSOufOCiOJtF8D\n\tEvkRgo8EXxifrOFcXGbBxF8g2m0YmUHwUHSNC7CKON5u2EhRh3lrK8h3nejNGdoXma\n\tdHcOf74SLV3jzq+7es5nLLp28xQBPDjJVymwRyiA=","Date":"Sat, 22 May 2021 02:59:42 +0300","From":"Laurent Pinchart ","To":"Kieran Bingham ","Message-ID":"","References":"<20210521092214.1160-1-laurent.pinchart@ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH] libcamera: log: Document LOG()\n\trestrictions","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]