[{"id":3003,"web_url":"https://patchwork.libcamera.org/comment/3003/","msgid":"<20191108112808.GF4866@pendragon.ideasonboard.com>","date":"2019-11-08T11:28:08","subject":"Re: [libcamera-devel] [PATCH 1/1] libcamera: replaced Meyer's\n\tsingleton","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hello Rynn Wu,\n\nThank you for the patch.\n\nOn Fri, Nov 08, 2019 at 04:22:33AM +0000, Rynn Wu (吳育恩) wrote:\n> Hi there,\n> \n> I found a Meyer’s singleton in IPAManager, basically Meyer’s singleton is\n> supposed to be removed since it causes some unexpected exceptions in Android\n> platforms while destroying the process.\n\nI wasn't aware of this issue, would you have a pointer to documentation\nexplaining why it causes problems specifically for Android ? Or is it a\ndesign pattern that should always be avoided ?\n\n> I replaced it by std::call_once and std::unique_ptr, here is the patch, please\n> kindly help to review it, thanks :-)\n\nYour mailer or mail server has corrupted the patch by adding blank\nlines and converting tabs to spaces, so I can't apply it properly. Is\nthis something you could fix ? Maybe by using git-send-email to send the\npatch ?\n\n> From 7efdb2e21263ff559cda8af3ec49721967ba8c60 Mon Sep 17 00:00:00 2001\n> \n> From: Rynn Wu \n> \n> Date: Fri, 8 Nov 2019 12:05:55 +0800\n> \n> Subject: [PATCH] libcamera: replaced Meyer's singleton\n> \n> \n> \n> Since destroying order of Meyer's singleton is not guarantee and\n> \n> there were some unexpected behaviors in Android platforms, it's better\n> \n> to replace it by other ways.\n> \n> \n> \n> Change-Id: Id2da20ea641f580cb3566f034cc9958bc391eac7\n\nChange-Id isn't applicable to libcamera upstream development, please\ndon't include it in patches.\n\n> Signed-off-by: Rynn Wu \n> \n> ---\n> \n> src/libcamera/include/ipa_manager.h | 8 ++++++++\n> \n> src/libcamera/ipa_manager.cpp | 12 ++++++++++--\n> \n> 2 files changed, 18 insertions(+), 2 deletions(-)\n> \n> \n> \n> diff --git a/src/libcamera/include/ipa_manager.h b/src/libcamera/include/\n> ipa_manager.h\n> \n> index 126f8ba..edaedf9 100644\n> \n> --- a/src/libcamera/include/ipa_manager.h\n> \n> +++ b/src/libcamera/include/ipa_manager.h\n> \n> @@ -7,6 +7,9 @@\n> \n> #ifndef __LIBCAMERA_IPA_MANAGER_H__\n> \n> #define __LIBCAMERA_IPA_MANAGER_H__\n> \n> +#include \n> \n> +#include \n> \n> +#include \n> \n> #include \n> \n> #include \n> \n> @@ -33,6 +36,11 @@ private:\n> \n> ~IPAManager();\n> \n> int addDir(const char *libDir);\n> \n> +\n> \n> +private:\n> \n> + static std::unique_ptr (IPAManager*)>> singleton_;\n> \n> + static std::once_flag singletonFlag_;\n> \n> +\n> \n> };\n> \n> } /* namespace libcamera */\n> \n> diff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp\n> \n> index f3180c0..8f60be4 100644\n> \n> --- a/src/libcamera/ipa_manager.cpp\n> \n> +++ b/src/libcamera/ipa_manager.cpp\n> \n> @@ -9,6 +9,7 @@\n> \n> #include \n> \n> #include \n> \n> +#include \n> \n> #include \n> \n> #include \n> \n> @@ -79,6 +80,9 @@ IPAManager::~IPAManager()\n> \n> delete module;\n> \n> }\n> \n> +std::unique_ptr>\n> IPAManager::singleton_;\n> \n> +std::once_flag IPAManager:: singletonFlag_;\n> \n> +\n> \n> /**\n> \n> * \\brief Retrieve the IPA manager instance\n> \n> *\n> \n> @@ -90,8 +94,12 @@ IPAManager::~IPAManager()\n> \n> */\n> \n> IPAManager *IPAManager::instance()\n> \n> {\n> \n> - static IPAManager ipaManager;\n> \n> - return &ipaManager;\n> \n> + std::call_once(IPAManager::singletonFlag_, [](){\n> \n> + /* never release, give an empty\n> customized deleter */\n\nThis causes a memory leak, which will show up in valgrind. We want to\navoid this, otherwise it gets difficult to tell true leaks apart from\nfalse positives in a valgrind report.\n\n> \n> + IPAManager::singleton_ =\n> std::unique_ptr>(\n> \n> + new IPAManager,\n> [](IPAManager*){});\n> \n> + });\n> \n> + return IPAManager::singleton_.get();\n> \n> }\n> \n> /**\n> \n> --\n> \n> 2.18.0\n> \n> \n> \n> ************* MEDIATEK Confidentiality Notice ********************\n> The information contained in this e-mail message (including any\n> attachments) may be confidential, proprietary, privileged, or otherwise\n> exempt from disclosure under applicable laws. It is intended to be\n> conveyed only to the designated recipient(s). Any use, dissemination,\n> distribution, printing, retaining or copying of this e-mail (including its\n> attachments) by unintended recipient(s) is strictly prohibited and may\n> be unlawful. If you are not an intended recipient of this e-mail, or believe\n> that you have received this e-mail in error, please notify the sender\n> immediately (by replying to this e-mail), delete any and all copies of\n> this e-mail (including any attachments) from your system, and do not\n> disclose the content of this e-mail to any other person. Thank you!\n\nThis prevents me from applying the patch, sorry. Please drop such\nheaders on upstream open-source contributions, as it explicitly states\nthat the content is confidential and can't be used.","headers":{"Return-Path":"","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0A7CD60C21\n\tfor ;\n\tFri, 8 Nov 2019 12:28:17 +0100 (CET)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 753CA31D;\n\tFri, 8 Nov 2019 12:28:17 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1573212497;\n\tbh=JKkxgf7sYtw/1yHIjJBUcPYVX0WN0U93LQb711/fRxQ=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=US3sgRc7v5yNZObmuIZy79SVwOpBUGHxmbvXa7lE4Vh3tlFvb3EYagHs1G8Yd4U48\n\tUOoPxrtzZRBPKx38gZ9Js7AkDD7fM7YPhrDecZhr+mR1BrnQYZKYzSdThlfQJdu2MQ\n\tV/4mVuMChPvMvEqYQSotLTlRdo2g1P9pX5pf9sUY=","Date":"Fri, 8 Nov 2019 13:28:08 +0200","From":"Laurent Pinchart ","To":"Rynn Wu =?utf-8?b?KOWQs+iCsuaBqSk=?= ","Cc":"\"libcamera-devel@lists.libcamera.org\"\n\t","Message-ID":"<20191108112808.GF4866@pendragon.ideasonboard.com>","References":"<39c4470d6fa04c17828f49a9afc8ffd9@mtkmbs07n1.mediatek.inc>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<39c4470d6fa04c17828f49a9afc8ffd9@mtkmbs07n1.mediatek.inc>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 1/1] libcamera: replaced Meyer's\n\tsingleton","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","X-List-Received-Date":"Fri, 08 Nov 2019 11:28:18 -0000"}}]