From patchwork Wed Jun 3 14:16:05 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paul Elder X-Patchwork-Id: 3905 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id C9C19610CD for ; Wed, 3 Jun 2020 16:16:23 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="LWhT9JBN"; dkim-atps=neutral Received: from emerald.amanokami.net (fs76eef344.knge213.ap.nuro.jp [118.238.243.68]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 96ED327C; Wed, 3 Jun 2020 16:16:22 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1591193783; bh=AIr7wRNotOTy7U1Ar79JAo4BPF97/S2I+/BM2hurd6c=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=LWhT9JBNdM7z3OFCOCCoiJT5tIT+WhzkPwaDQMOh1tN7WZl/mEFd6DqFCrJsbiTP6 oj/W+xvPqr5J5Ro/M6koh0/7n3xagvOWLFeRkcxHkYxOXjopWpGoVPha5silPTPfxi C0r0laQdcMe3AoMf9vTPrrRHpjkf5jHGtmICsK48= From: Paul Elder To: libcamera-devel@lists.libcamera.org Date: Wed, 3 Jun 2020 23:16:05 +0900 Message-Id: <20200603141609.18584-2-paul.elder@ideasonboard.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20200603141609.18584-1-paul.elder@ideasonboard.com> References: <20200603141609.18584-1-paul.elder@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 1/5] IPAManager: make IPAManager lifetime explicitly managed 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: , X-List-Received-Date: Wed, 03 Jun 2020 14:16:24 -0000 If any ipa_context instances are destroyed before the IPAManager is destroyed, then a segfault will occur when the IPAManager is deconstructed, as it tries to destroy all of the IPAs that it manages. Fix this by making the lifetime of the IPAManager explicit, and make the CameraManager construct and deconstruct (automatically, via a unique pointer) the IPAManager. Also update the IPA interface test to do the construction and deconstruction of the IPAManager, as it does not use the CameraManager. Signed-off-by: Paul Elder --- include/libcamera/camera_manager.h | 4 ++++ include/libcamera/internal/ipa_manager.h | 7 ++++--- src/libcamera/camera_manager.cpp | 4 +++- src/libcamera/ipa_manager.cpp | 13 +++++++++++-- test/ipa/ipa_interface_test.cpp | 5 +++++ 5 files changed, 27 insertions(+), 6 deletions(-) diff --git a/include/libcamera/camera_manager.h b/include/libcamera/camera_manager.h index 079f848a..8f9398fa 100644 --- a/include/libcamera/camera_manager.h +++ b/include/libcamera/camera_manager.h @@ -14,6 +14,8 @@ #include +#include "libcamera/internal/ipa_manager.h" + namespace libcamera { class Camera; @@ -48,6 +50,8 @@ private: class Private; std::unique_ptr p_; + + std::unique_ptr ipaManager_; }; } /* namespace libcamera */ diff --git a/include/libcamera/internal/ipa_manager.h b/include/libcamera/internal/ipa_manager.h index 2412d757..65303d6d 100644 --- a/include/libcamera/internal/ipa_manager.h +++ b/include/libcamera/internal/ipa_manager.h @@ -22,6 +22,9 @@ namespace libcamera { class IPAManager { public: + IPAManager(); + ~IPAManager(); + static IPAManager *instance(); std::unique_ptr createIPA(PipelineHandler *pipe, @@ -30,9 +33,7 @@ public: private: std::vector modules_; - - IPAManager(); - ~IPAManager(); + static IPAManager *self_; void parseDir(const char *libDir, unsigned int maxDepth, std::vector &files); diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp index 849377ad..d19348df 100644 --- a/src/libcamera/camera_manager.cpp +++ b/src/libcamera/camera_manager.cpp @@ -15,6 +15,7 @@ #include "libcamera/internal/device_enumerator.h" #include "libcamera/internal/event_dispatcher_poll.h" +#include "libcamera/internal/ipa_manager.h" #include "libcamera/internal/log.h" #include "libcamera/internal/pipeline_handler.h" #include "libcamera/internal/thread.h" @@ -246,7 +247,8 @@ void CameraManager::Private::removeCamera(Camera *camera) CameraManager *CameraManager::self_ = nullptr; CameraManager::CameraManager() - : p_(new CameraManager::Private(this)) + : p_(new CameraManager::Private(this)), ipaManager_(new IPAManager()) + { if (self_) LOG(Camera, Fatal) diff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp index 505cf610..74112cde 100644 --- a/src/libcamera/ipa_manager.cpp +++ b/src/libcamera/ipa_manager.cpp @@ -93,8 +93,14 @@ LOG_DEFINE_CATEGORY(IPAManager) * plain C API, or to transmit the data to the isolated process through IPC. */ +IPAManager *IPAManager::self_ = nullptr; + IPAManager::IPAManager() { + if (self_) + LOG(IPAManager, Fatal) + << "Multiple IPAManager objects are not allowed"; + unsigned int ipaCount = 0; /* User-specified paths take precedence. */ @@ -134,12 +140,16 @@ IPAManager::IPAManager() if (!ipaCount) LOG(IPAManager, Warning) << "No IPA found in '" IPA_MODULE_DIR "'"; + + self_ = this; } IPAManager::~IPAManager() { for (IPAModule *module : modules_) delete module; + + self_ = nullptr; } /** @@ -153,8 +163,7 @@ IPAManager::~IPAManager() */ IPAManager *IPAManager::instance() { - static IPAManager ipaManager; - return &ipaManager; + return self_; } /** diff --git a/test/ipa/ipa_interface_test.cpp b/test/ipa/ipa_interface_test.cpp index 2f02af49..153493ba 100644 --- a/test/ipa/ipa_interface_test.cpp +++ b/test/ipa/ipa_interface_test.cpp @@ -39,11 +39,15 @@ public: ~IPAInterfaceTest() { delete notifier_; + ipa_.reset(); + ipaManager_.reset(); } protected: int init() override { + ipaManager_ = make_unique(); + /* Create a pipeline handler for vimc. */ std::vector &factories = PipelineHandlerFactory::factories(); @@ -161,6 +165,7 @@ private: std::shared_ptr pipe_; std::unique_ptr ipa_; + std::unique_ptr ipaManager_; enum IPAOperationCode trace_; EventNotifier *notifier_; int fd_;