From patchwork Fri Jun 5 09:01:00 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Paul Elder X-Patchwork-Id: 3936 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id C5F2661027 for ; Fri, 5 Jun 2020 11:01:17 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="HrHlCjzR"; 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 A151F27C; Fri, 5 Jun 2020 11:01:16 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1591347677; bh=7zrH4zk88iLncnUuGh1zlgEHRYvx0Sw1lvltKXxCbvA=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=HrHlCjzR8AFfyUijcYSh2DzR+Woevg5Ffe86t5LY8k4prnFGyOjjdL9R5g8StaMZg K+grA0kBAS/AlPr13S/xJ/1XRUYmzWylTd8m0D1SybLbTUOVMcv+sZK5nwXla40Y2U LPHUHndWLjmVKi4jeEKmOkKYP0qLGhyiHQhhkXBA= From: Paul Elder To: libcamera-devel@lists.libcamera.org Date: Fri, 5 Jun 2020 18:01:00 +0900 Message-Id: <20200605090106.15424-2-paul.elder@ideasonboard.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20200605090106.15424-1-paul.elder@ideasonboard.com> References: <20200605090106.15424-1-paul.elder@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 1/7] 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: Fri, 05 Jun 2020 09:01:18 -0000 If any ipa_context instances are destroyed after the IPAManager is destroyed, then a segfault will occur, since the modules have been unloaded by the IPAManager and the context function pointers have been freed. 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 Reviewed-by: Laurent Pinchart Reviewed-by: Niklas Söderlund --- Changes in v2: Move the IPAManager instance into CameraManager::Private from CameraManager --- include/libcamera/camera_manager.h | 1 - include/libcamera/internal/ipa_manager.h | 6 ++++-- src/libcamera/camera_manager.cpp | 3 +++ src/libcamera/ipa_manager.cpp | 20 ++++++++++++++++++-- test/ipa/ipa_interface_test.cpp | 5 +++++ 5 files changed, 30 insertions(+), 5 deletions(-) diff --git a/include/libcamera/camera_manager.h b/include/libcamera/camera_manager.h index 079f848a..00658a70 100644 --- a/include/libcamera/camera_manager.h +++ b/include/libcamera/camera_manager.h @@ -18,7 +18,6 @@ namespace libcamera { class Camera; class EventDispatcher; - class CameraManager : public Object { public: diff --git a/include/libcamera/internal/ipa_manager.h b/include/libcamera/internal/ipa_manager.h index 16d74291..43f700d3 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, @@ -29,8 +32,7 @@ public: uint32_t minVersion); private: - 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..da806fa7 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" @@ -63,6 +64,8 @@ private: std::vector> pipes_; std::unique_ptr enumerator_; + + IPAManager ipaManager_; }; CameraManager::Private::Private(CameraManager *cm) diff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp index 505cf610..abb681a3 100644 --- a/src/libcamera/ipa_manager.cpp +++ b/src/libcamera/ipa_manager.cpp @@ -93,8 +93,21 @@ LOG_DEFINE_CATEGORY(IPAManager) * plain C API, or to transmit the data to the isolated process through IPC. */ +IPAManager *IPAManager::self_ = nullptr; + +/** + * \brief Construct an IPAManager instance + * + * The IPAManager class is meant to only be instantiated once, by the + * CameraManager. Pipeline handlers shall use the instance() function to access + * the IPAManager instance. + */ IPAManager::IPAManager() { + if (self_) + LOG(IPAManager, Fatal) + << "Multiple IPAManager objects are not allowed"; + unsigned int ipaCount = 0; /* User-specified paths take precedence. */ @@ -134,12 +147,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 +170,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_;