From patchwork Mon Jul 28 11:36:36 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= X-Patchwork-Id: 23991 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 28C5BC3237 for ; Mon, 28 Jul 2025 11:36:54 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 6DBBB6915E; Mon, 28 Jul 2025 13:36:50 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="hmZx+KAs"; dkim-atps=neutral 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 0020369153 for ; Mon, 28 Jul 2025 13:36:45 +0200 (CEST) Received: from pb-laptop.local (185.221.140.39.nat.pool.zt.hu [185.221.140.39]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id BCC786DC; Mon, 28 Jul 2025 13:36:03 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1753702563; bh=c5xLt+wLhF3jZ4xl/trVmwF/Kl0iE0AhjOPSxn5pijs=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=hmZx+KAswCSFXPrkFKfROZPrB9X4HMRSpEvhNUbHTcB2XxYXVhYPxybecOezkfw5y bxYpAn84f9eSym3ydXTgCUkBsWo4H/WUlaTJPhO4rdACtZM7nQC7YieG7Sb1GU3UJS HeL2r06aHX5G3f0u9a1aydapbxVJ7n2ggSrznWd8= From: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= To: libcamera-devel@lists.libcamera.org Cc: Kieran Bingham , Laurent Pinchart Subject: [PATCH v6 1/6] libcamera: process: Use `pid_` member to decide if running Date: Mon, 28 Jul 2025 13:36:36 +0200 Message-ID: <20250728113641.238256-2-barnabas.pocze@ideasonboard.com> X-Mailer: git-send-email 2.50.1 In-Reply-To: <20250728113641.238256-1-barnabas.pocze@ideasonboard.com> References: <20250728113641.238256-1-barnabas.pocze@ideasonboard.com> MIME-Version: 1.0 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: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Instead of using a separate member variable, use `pid_ > 0` to determine if the process is still running. Previously the value of `pid_` was not reset to -1 when the process terminated, but since it is only meaningful while the process is running, reset it to -1 in `Process::died()`. Neither `pid_` nor `running_` are exposed, so this change has no effect on the public interface or observable behaviour. Signed-off-by: Barnabás Pőcze Reviewed-by: Kieran Bingham Reviewed-by: Laurent Pinchart --- include/libcamera/internal/process.h | 1 - src/libcamera/process.cpp | 8 +++----- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/include/libcamera/internal/process.h b/include/libcamera/internal/process.h index 6c34aef2f..891e78947 100644 --- a/include/libcamera/internal/process.h +++ b/include/libcamera/internal/process.h @@ -50,7 +50,6 @@ private: void died(int wstatus); pid_t pid_; - bool running_; enum ExitStatus exitStatus_; int exitCode_; diff --git a/src/libcamera/process.cpp b/src/libcamera/process.cpp index 0eae68072..a7add0b17 100644 --- a/src/libcamera/process.cpp +++ b/src/libcamera/process.cpp @@ -208,7 +208,7 @@ const struct sigaction &ProcessManager::oldsa() const */ Process::Process() - : pid_(-1), running_(false), exitStatus_(NotExited), exitCode_(0) + : pid_(-1), exitStatus_(NotExited), exitCode_(0) { } @@ -240,7 +240,7 @@ int Process::start(const std::string &path, { int ret; - if (running_) + if (pid_ > 0) return -EBUSY; for (int fd : fds) { @@ -257,8 +257,6 @@ int Process::start(const std::string &path, pid_ = childPid; ProcessManager::instance()->registerProcess(this); - running_ = true; - return 0; } else { if (isolate()) @@ -348,7 +346,7 @@ int Process::isolate() */ void Process::died(int wstatus) { - running_ = false; + pid_ = -1; exitStatus_ = WIFEXITED(wstatus) ? NormalExit : SignalExit; exitCode_ = exitStatus_ == NormalExit ? WEXITSTATUS(wstatus) : -1; From patchwork Mon Jul 28 11:36:37 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= X-Patchwork-Id: 23993 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id A7C58C332A for ; Mon, 28 Jul 2025 11:36:58 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id F323A6916F; Mon, 28 Jul 2025 13:36:54 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="eCHginpo"; dkim-atps=neutral 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 858136904C for ; Mon, 28 Jul 2025 13:36:46 +0200 (CEST) Received: from pb-laptop.local (185.221.140.39.nat.pool.zt.hu [185.221.140.39]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 1B51E928; Mon, 28 Jul 2025 13:36:04 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1753702564; bh=oeuWLwy6WcxkXKfyfTY6/CUeuECx5MywK/VsX7tn5R4=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=eCHginpofMQp/gqASZghFia7is279CQCDr6s+SeUFIJfe6jOLAJXBGfPXhQ6SJQRI EomdOB0WqY8sn2z9MPmfU4ynLBDu2GkcY5DWu9Y8vy54tpP+Dpvp0FdeOC2cHU6i53 H4JhKI36/FrSqayFZYiSKs7H+elViSlLCe95lHNw= From: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= To: libcamera-devel@lists.libcamera.org Cc: Kieran Bingham , Laurent Pinchart Subject: [PATCH v6 2/6] libcamera: process: start(): Use span instead of vector Date: Mon, 28 Jul 2025 13:36:37 +0200 Message-ID: <20250728113641.238256-3-barnabas.pocze@ideasonboard.com> X-Mailer: git-send-email 2.50.1 In-Reply-To: <20250728113641.238256-1-barnabas.pocze@ideasonboard.com> References: <20250728113641.238256-1-barnabas.pocze@ideasonboard.com> MIME-Version: 1.0 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: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Use a span instead of a const reference to a vector, this does not change the behaviour and allows e.g. arrays to be used to hold arguments/file descriptors. Signed-off-by: Barnabás Pőcze Reviewed-by: Kieran Bingham Reviewed-by: Laurent Pinchart --- include/libcamera/internal/process.h | 6 +++--- src/libcamera/ipc_pipe_unixsocket.cpp | 9 +++------ src/libcamera/process.cpp | 6 +++--- test/process/process_test.cpp | 5 ++--- 4 files changed, 11 insertions(+), 15 deletions(-) diff --git a/include/libcamera/internal/process.h b/include/libcamera/internal/process.h index 891e78947..307c809f7 100644 --- a/include/libcamera/internal/process.h +++ b/include/libcamera/internal/process.h @@ -9,10 +9,10 @@ #include #include -#include #include #include +#include #include namespace libcamera { @@ -32,8 +32,8 @@ public: ~Process(); int start(const std::string &path, - const std::vector &args = std::vector(), - const std::vector &fds = std::vector()); + Span args = {}, + Span fds = {}); ExitStatus exitStatus() const { return exitStatus_; } int exitCode() const { return exitCode_; } diff --git a/src/libcamera/ipc_pipe_unixsocket.cpp b/src/libcamera/ipc_pipe_unixsocket.cpp index 668ec73b9..7ee7cac79 100644 --- a/src/libcamera/ipc_pipe_unixsocket.cpp +++ b/src/libcamera/ipc_pipe_unixsocket.cpp @@ -28,10 +28,6 @@ IPCPipeUnixSocket::IPCPipeUnixSocket(const char *ipaModulePath, const char *ipaProxyWorkerPath) : IPCPipe() { - std::vector fds; - std::vector args; - args.push_back(ipaModulePath); - socket_ = std::make_unique(); UniqueFD fd = socket_->create(); if (!fd.isValid()) { @@ -39,8 +35,9 @@ IPCPipeUnixSocket::IPCPipeUnixSocket(const char *ipaModulePath, return; } socket_->readyRead.connect(this, &IPCPipeUnixSocket::readyRead); - args.push_back(std::to_string(fd.get())); - fds.push_back(fd.get()); + + std::array args{ std::string(ipaModulePath), std::to_string(fd.get()) }; + std::array fds{ fd.get() }; proc_ = std::make_unique(); int ret = proc_->start(ipaProxyWorkerPath, args, fds); diff --git a/src/libcamera/process.cpp b/src/libcamera/process.cpp index a7add0b17..479163e83 100644 --- a/src/libcamera/process.cpp +++ b/src/libcamera/process.cpp @@ -235,8 +235,8 @@ Process::~Process() * or a negative error code otherwise */ int Process::start(const std::string &path, - const std::vector &args, - const std::vector &fds) + Span args, + Span fds) { int ret; @@ -262,7 +262,7 @@ int Process::start(const std::string &path, if (isolate()) _exit(EXIT_FAILURE); - std::vector v(fds); + std::vector v(fds.begin(), fds.end()); v.push_back(STDERR_FILENO); closeAllFdsExcept(v); diff --git a/test/process/process_test.cpp b/test/process/process_test.cpp index e9f5e7e9b..a88d8fef1 100644 --- a/test/process/process_test.cpp +++ b/test/process/process_test.cpp @@ -5,9 +5,9 @@ * Process test */ +#include #include #include -#include #include #include @@ -48,8 +48,7 @@ protected: Timer timeout; int exitCode = 42; - vector args; - args.push_back(to_string(exitCode)); + std::array args{ to_string(exitCode) }; proc_.finished.connect(this, &ProcessTest::procFinished); /* Test that kill() on an unstarted process is safe. */ From patchwork Mon Jul 28 11:36:38 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= X-Patchwork-Id: 23992 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id A6BE5C3323 for ; Mon, 28 Jul 2025 11:36:56 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 07EC469153; Mon, 28 Jul 2025 13:36:54 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="GlcZmVFi"; dkim-atps=neutral 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 571A769158 for ; Mon, 28 Jul 2025 13:36:46 +0200 (CEST) Received: from pb-laptop.local (185.221.140.39.nat.pool.zt.hu [185.221.140.39]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 6E4404A4 for ; Mon, 28 Jul 2025 13:36:04 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1753702564; bh=6amt1pfSCZMrGeT/snXCoeIbXZlm0bIT7dtXjOLqdwU=; h=From:To:Subject:Date:In-Reply-To:References:From; b=GlcZmVFi8/Yhl/NP38CEnQZ5tB4kDy9D9PTvXboJmvb8KT6bkRdkTe/9iaZ1NDOa6 m3BHQkMoqzhMKAZEYRbIGCYr4otMHje3ZF//Ry++4VP+zZFC69EeYlFJ6fU3r8GzZu lK85pYd6HNgvymmE9E9lGs9buDbOVNNt3rQQPZbc= From: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= To: libcamera-devel@lists.libcamera.org Subject: [PATCH v6 3/6] libcamera: process: closeAllFdsExcept(): Take vector by value Date: Mon, 28 Jul 2025 13:36:38 +0200 Message-ID: <20250728113641.238256-4-barnabas.pocze@ideasonboard.com> X-Mailer: git-send-email 2.50.1 In-Reply-To: <20250728113641.238256-1-barnabas.pocze@ideasonboard.com> References: <20250728113641.238256-1-barnabas.pocze@ideasonboard.com> MIME-Version: 1.0 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: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Instead of creating a new vector, take the vector by value to make it possible for the caller to use move construction when calling the function. Signed-off-by: Barnabás Pőcze Reviewed-by: Laurent Pinchart --- include/libcamera/internal/process.h | 2 +- src/libcamera/process.cpp | 5 ++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/include/libcamera/internal/process.h b/include/libcamera/internal/process.h index 307c809f7..4ab846b24 100644 --- a/include/libcamera/internal/process.h +++ b/include/libcamera/internal/process.h @@ -45,7 +45,7 @@ public: private: LIBCAMERA_DISABLE_COPY_AND_MOVE(Process) - void closeAllFdsExcept(const std::vector &fds); + void closeAllFdsExcept(std::vector v); int isolate(); void died(int wstatus); diff --git a/src/libcamera/process.cpp b/src/libcamera/process.cpp index 479163e83..5193386ce 100644 --- a/src/libcamera/process.cpp +++ b/src/libcamera/process.cpp @@ -264,7 +264,7 @@ int Process::start(const std::string &path, std::vector v(fds.begin(), fds.end()); v.push_back(STDERR_FILENO); - closeAllFdsExcept(v); + closeAllFdsExcept(std::move(v)); const auto tryDevNullLowestFd = [](int expected, int oflag) { int fd = open("/dev/null", oflag); @@ -296,9 +296,8 @@ int Process::start(const std::string &path, } } -void Process::closeAllFdsExcept(const std::vector &fds) +void Process::closeAllFdsExcept(std::vector v) { - std::vector v(fds); sort(v.begin(), v.end()); ASSERT(v.empty() || v.front() >= 0); From patchwork Mon Jul 28 11:36:39 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= X-Patchwork-Id: 23994 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id E2E37C332B for ; Mon, 28 Jul 2025 11:37:00 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 8AF1F69164; Mon, 28 Jul 2025 13:36:56 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="QXFEP342"; dkim-atps=neutral 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 A908169159 for ; Mon, 28 Jul 2025 13:36:46 +0200 (CEST) Received: from pb-laptop.local (185.221.140.39.nat.pool.zt.hu [185.221.140.39]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id ABC36C72; Mon, 28 Jul 2025 13:36:04 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1753702564; bh=4/BE+YfFh43F4ZcvYJCKXIV+ljfVNpilAlG1SR/dzaE=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=QXFEP342tao8iA4ExjIKNa3JUgiwp+DgBZG1sOMWJx3ITLp1eHxXxIJOUI2oUhof/ d1Z5Eqy8my6NWE/nqgp46eITFfQ0mREvYvIVvJ25YA6mhoFSG7Cwb4tsyK4aW4KgCj PmuG0BHTNEGN7yoRwfVCwR2j7aejVOXTqMe/9Xx4= From: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= To: libcamera-devel@lists.libcamera.org Cc: Laurent Pinchart , Kieran Bingham Subject: [PATCH v6 4/6] libcamera: process: Move `closeAllFdsExcept()` Date: Mon, 28 Jul 2025 13:36:39 +0200 Message-ID: <20250728113641.238256-5-barnabas.pocze@ideasonboard.com> X-Mailer: git-send-email 2.50.1 In-Reply-To: <20250728113641.238256-1-barnabas.pocze@ideasonboard.com> References: <20250728113641.238256-1-barnabas.pocze@ideasonboard.com> MIME-Version: 1.0 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: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Remove `closeAllFdsExcept()` from the `Process` class and have it as a local function since it is not needed "outside" and does not depend on any part of the `Process` class. Signed-off-by: Barnabás Pőcze Reviewed-by: Laurent Pinchart Reviewed-by: Kieran Bingham --- include/libcamera/internal/process.h | 1 - src/libcamera/process.cpp | 54 ++++++++++++++-------------- 2 files changed, 27 insertions(+), 28 deletions(-) diff --git a/include/libcamera/internal/process.h b/include/libcamera/internal/process.h index 4ab846b24..e55d11fa3 100644 --- a/include/libcamera/internal/process.h +++ b/include/libcamera/internal/process.h @@ -45,7 +45,6 @@ public: private: LIBCAMERA_DISABLE_COPY_AND_MOVE(Process) - void closeAllFdsExcept(std::vector v); int isolate(); void died(int wstatus); diff --git a/src/libcamera/process.cpp b/src/libcamera/process.cpp index 5193386ce..4b87c5591 100644 --- a/src/libcamera/process.cpp +++ b/src/libcamera/process.cpp @@ -63,6 +63,33 @@ void sigact(int signal, siginfo_t *info, void *ucontext) } } +void closeAllFdsExcept(std::vector v) +{ + sort(v.begin(), v.end()); + + ASSERT(v.empty() || v.front() >= 0); + + DIR *dir = opendir("/proc/self/fd"); + if (!dir) + return; + + int dfd = dirfd(dir); + + struct dirent *ent; + while ((ent = readdir(dir)) != nullptr) { + char *endp; + int fd = strtoul(ent->d_name, &endp, 10); + if (*endp) + continue; + + if (fd >= 0 && fd != dfd && + !std::binary_search(v.begin(), v.end(), fd)) + close(fd); + } + + closedir(dir); +} + } /* namespace */ void ProcessManager::sighandler() @@ -296,33 +323,6 @@ int Process::start(const std::string &path, } } -void Process::closeAllFdsExcept(std::vector v) -{ - sort(v.begin(), v.end()); - - ASSERT(v.empty() || v.front() >= 0); - - DIR *dir = opendir("/proc/self/fd"); - if (!dir) - return; - - int dfd = dirfd(dir); - - struct dirent *ent; - while ((ent = readdir(dir)) != nullptr) { - char *endp; - int fd = strtoul(ent->d_name, &endp, 10); - if (*endp) - continue; - - if (fd >= 0 && fd != dfd && - !std::binary_search(v.begin(), v.end(), fd)) - close(fd); - } - - closedir(dir); -} - int Process::isolate() { int ret = unshare(CLONE_NEWUSER | CLONE_NEWNET); From patchwork Mon Jul 28 11:36:40 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= X-Patchwork-Id: 23995 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id A8BC4C332C for ; Mon, 28 Jul 2025 11:37:02 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id B56196917B; Mon, 28 Jul 2025 13:36:58 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="P4d0S07j"; dkim-atps=neutral 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 05BF06915B for ; Mon, 28 Jul 2025 13:36:47 +0200 (CEST) Received: from pb-laptop.local (185.221.140.39.nat.pool.zt.hu [185.221.140.39]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 081724A4; Mon, 28 Jul 2025 13:36:04 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1753702565; bh=zU468YZgi0yylox2Et+YlkCakIhOIDFwylOmeVyQzb4=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=P4d0S07jzrLVhBsrQq6e1i0+/CkCVwjH7XtH4MxOgex7aEEprjL57BwQGKY00EmW+ 89ZOkEC0aKRclc1KQ+T041Qc/Bgv00vrxPQu28LUgq57n5DyhD5gz8lpfUuHVRNinX 3tS0s/W2E9l5wAnj4D/9LeHf0Q1Hcqp2JRR1+xlI= From: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= To: libcamera-devel@lists.libcamera.org Cc: Laurent Pinchart , Kieran Bingham Subject: [PATCH v6 5/6] libcamera: process: Use `close_range()` when available Date: Mon, 28 Jul 2025 13:36:40 +0200 Message-ID: <20250728113641.238256-6-barnabas.pocze@ideasonboard.com> X-Mailer: git-send-email 2.50.1 In-Reply-To: <20250728113641.238256-1-barnabas.pocze@ideasonboard.com> References: <20250728113641.238256-1-barnabas.pocze@ideasonboard.com> MIME-Version: 1.0 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: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Use the `close_range()` system call when available as it is simpler and faster than iterating `/proc/self/fd/`. Signed-off-by: Barnabás Pőcze Reviewed-by: Laurent Pinchart Reviewed-by: Kieran Bingham --- meson.build | 4 ++++ src/libcamera/process.cpp | 25 +++++++++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/meson.build b/meson.build index 4ed8017eb..6fc4504ab 100644 --- a/meson.build +++ b/meson.build @@ -74,6 +74,10 @@ cc = meson.get_compiler('c') cxx = meson.get_compiler('cpp') config_h = configuration_data() +if cc.has_header_symbol('unistd.h', 'close_range', prefix : '#define _GNU_SOURCE') + config_h.set('HAVE_CLOSE_RANGE', 1) +endif + if cc.has_header_symbol('fcntl.h', 'F_ADD_SEALS', prefix : '#define _GNU_SOURCE') config_h.set('HAVE_FILE_SEALS', 1) endif diff --git a/src/libcamera/process.cpp b/src/libcamera/process.cpp index 4b87c5591..ce6e4c380 100644 --- a/src/libcamera/process.cpp +++ b/src/libcamera/process.cpp @@ -69,6 +69,31 @@ void closeAllFdsExcept(std::vector v) ASSERT(v.empty() || v.front() >= 0); +#if HAVE_CLOSE_RANGE + /* + * At the moment libcamera does not require at least Linux 5.9, + * which introduced the `close_range()` system call, so a runtime + * check is also needed to make sure that it is supported. + */ + static const bool hasCloseRange = [] { + return close_range(~0u, 0, 0) < 0 && errno == EINVAL; + }(); + + if (hasCloseRange) { + unsigned int prev = 0; + + for (unsigned int curr : v) { + ASSERT(prev <= curr + 1); + if (prev < curr) + close_range(prev, curr - 1, 0); + prev = curr + 1; + } + + close_range(prev, ~0u, 0); + return; + } +#endif + DIR *dir = opendir("/proc/self/fd"); if (!dir) return; From patchwork Mon Jul 28 11:36:41 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= X-Patchwork-Id: 23996 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 445B8C332D for ; Mon, 28 Jul 2025 11:37:04 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 77AE569166; Mon, 28 Jul 2025 13:37:01 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="lUO94To7"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 54B4869156 for ; Mon, 28 Jul 2025 13:36:47 +0200 (CEST) Received: from pb-laptop.local (185.221.140.39.nat.pool.zt.hu [185.221.140.39]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 5D61AF04; Mon, 28 Jul 2025 13:36:05 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1753702565; bh=HdGVmHtZmISTLUWBO6S9V+zq0ItNCWWBxgMgHs1unZI=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=lUO94To7I6dUF1YUK8D5dOg7Qf6GrYnsMofvY/1//2tfIYWkBOYvaLluqtPkgutyv THsHm0O6NUP0axXM1qo3ZhROs3FYdD1eYFvG4qcQyWIIuAEzc4ZwfARf0/xbzibQNX 0o0zZftlzBbMkwNZkzV+5wwNIHlfFxGtXUMyzmBo= From: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= To: libcamera-devel@lists.libcamera.org Cc: Laurent Pinchart Subject: [PATCH v6 6/6] libcamera: process: Remove `ProcessManager` singleton Date: Mon, 28 Jul 2025 13:36:41 +0200 Message-ID: <20250728113641.238256-7-barnabas.pocze@ideasonboard.com> X-Mailer: git-send-email 2.50.1 In-Reply-To: <20250728113641.238256-1-barnabas.pocze@ideasonboard.com> References: <20250728113641.238256-1-barnabas.pocze@ideasonboard.com> MIME-Version: 1.0 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: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" The `ProcessManager` is a singleton class to handle `SIGCHLD` signals and report the exit status to the particular `Process` instance. However, having a singleton in a library is not favourable and it is even less favourable if it installs a signal handler. Using pidfd it is possible to avoid the need for the signal handler; and the `Process` objects can watch their pidfd themselves, eliminating the need for the `ProcessManager` class altogether. `P_PIDFD` for `waitid()` was introduced in Linux 5.4, so this change raises the minimum supported kernel version. `clone3()`, `CLONE_PIDFD`, `pidfd_send_signal()` were all introduced earlier. Furthermore, the call to the `unshare()` system call can be removed as those options can be passed to `clone3()` directly. Signed-off-by: Barnabás Pőcze Reviewed-by: Laurent Pinchart --- include/libcamera/internal/camera_manager.h | 1 - include/libcamera/internal/process.h | 34 +-- meson.build | 2 +- src/libcamera/process.cpp | 246 +++++--------------- test/ipc/unixsocket_ipc.cpp | 2 - test/log/log_process.cpp | 2 - test/process/process_test.cpp | 2 - 7 files changed, 61 insertions(+), 228 deletions(-) diff --git a/include/libcamera/internal/camera_manager.h b/include/libcamera/internal/camera_manager.h index 0150ca61f..5dfbe1f65 100644 --- a/include/libcamera/internal/camera_manager.h +++ b/include/libcamera/internal/camera_manager.h @@ -65,7 +65,6 @@ private: std::unique_ptr enumerator_; std::unique_ptr ipaManager_; - ProcessManager processManager_; }; } /* namespace libcamera */ diff --git a/include/libcamera/internal/process.h b/include/libcamera/internal/process.h index e55d11fa3..27d4b9258 100644 --- a/include/libcamera/internal/process.h +++ b/include/libcamera/internal/process.h @@ -7,7 +7,6 @@ #pragma once -#include #include #include @@ -45,41 +44,14 @@ public: private: LIBCAMERA_DISABLE_COPY_AND_MOVE(Process) - int isolate(); - void died(int wstatus); + void onPidfdNotify(); pid_t pid_; enum ExitStatus exitStatus_; int exitCode_; - friend class ProcessManager; -}; - -class ProcessManager -{ -public: - ProcessManager(); - ~ProcessManager(); - - void registerProcess(Process *proc); - - static ProcessManager *instance(); - - int writePipe() const; - - const struct sigaction &oldsa() const; - -private: - static ProcessManager *self_; - - void sighandler(); - - std::list processes_; - - struct sigaction oldsa_; - - EventNotifier *sigEvent_; - UniqueFD pipe_[2]; + UniqueFD pidfd_; + std::unique_ptr pidfdNotify_; }; } /* namespace libcamera */ diff --git a/meson.build b/meson.build index 6fc4504ab..47efdfcd4 100644 --- a/meson.build +++ b/meson.build @@ -265,7 +265,7 @@ subdir('Documentation') subdir('test') if not meson.is_cross_build() - kernel_version_req = '>= 5.0.0' + kernel_version_req = '>= 5.4.0' kernel_version = run_command('uname', '-r', check : true).stdout().strip() if not kernel_version.version_compare(kernel_version_req) warning('The current running kernel version @0@ is too old to run libcamera.' diff --git a/src/libcamera/process.cpp b/src/libcamera/process.cpp index ce6e4c380..3090d7568 100644 --- a/src/libcamera/process.cpp +++ b/src/libcamera/process.cpp @@ -10,15 +10,19 @@ #include #include #include -#include #include #include #include +#include #include #include #include +#include #include +#include +#include /* glibc only provides `P_PIDFD` in `sys/wait.h` since 2.36 */ + #include #include #include @@ -32,37 +36,8 @@ namespace libcamera { LOG_DEFINE_CATEGORY(Process) -/** - * \class ProcessManager - * \brief Manager of processes - * - * The ProcessManager singleton keeps track of all created Process instances, - * and manages the signal handling involved in terminating processes. - */ - namespace { -void sigact(int signal, siginfo_t *info, void *ucontext) -{ -#pragma GCC diagnostic push -#pragma GCC diagnostic ignored "-Wunused-result" - /* - * We're in a signal handler so we can't log any message, and we need - * to continue anyway. - */ - char data = 0; - write(ProcessManager::instance()->writePipe(), &data, sizeof(data)); -#pragma GCC diagnostic pop - - const struct sigaction &oldsa = ProcessManager::instance()->oldsa(); - if (oldsa.sa_flags & SA_SIGINFO) { - oldsa.sa_sigaction(signal, info, ucontext); - } else { - if (oldsa.sa_handler != SIG_IGN && oldsa.sa_handler != SIG_DFL) - oldsa.sa_handler(signal); - } -} - void closeAllFdsExcept(std::vector v) { sort(v.begin(), v.end()); @@ -117,129 +92,6 @@ void closeAllFdsExcept(std::vector v) } /* namespace */ -void ProcessManager::sighandler() -{ - char data; - ssize_t ret = read(pipe_[0].get(), &data, sizeof(data)); - if (ret < 0) { - LOG(Process, Error) - << "Failed to read byte from signal handler pipe"; - return; - } - - for (auto it = processes_.begin(); it != processes_.end();) { - Process *process = *it; - - int wstatus; - pid_t pid = waitpid(process->pid_, &wstatus, WNOHANG); - if (process->pid_ != pid) { - ++it; - continue; - } - - it = processes_.erase(it); - process->died(wstatus); - } -} - -/** - * \brief Register process with process manager - * \param[in] proc Process to register - * - * This function registers the \a proc with the process manager. It - * shall be called by the parent process after successfully forking, in - * order to let the parent signal process termination. - */ -void ProcessManager::registerProcess(Process *proc) -{ - processes_.push_back(proc); -} - -ProcessManager *ProcessManager::self_ = nullptr; - -/** - * \brief Construct a ProcessManager instance - * - * The ProcessManager class is meant to only be instantiated once, by the - * CameraManager. - */ -ProcessManager::ProcessManager() -{ - if (self_) - LOG(Process, Fatal) - << "Multiple ProcessManager objects are not allowed"; - - sigaction(SIGCHLD, NULL, &oldsa_); - - struct sigaction sa; - memset(&sa, 0, sizeof(sa)); - sa.sa_sigaction = &sigact; - memcpy(&sa.sa_mask, &oldsa_.sa_mask, sizeof(sa.sa_mask)); - sigaddset(&sa.sa_mask, SIGCHLD); - sa.sa_flags = oldsa_.sa_flags | SA_SIGINFO; - - sigaction(SIGCHLD, &sa, NULL); - - int pipe[2]; - if (pipe2(pipe, O_CLOEXEC | O_DIRECT | O_NONBLOCK)) - LOG(Process, Fatal) - << "Failed to initialize pipe for signal handling"; - - pipe_[0] = UniqueFD(pipe[0]); - pipe_[1] = UniqueFD(pipe[1]); - - sigEvent_ = new EventNotifier(pipe_[0].get(), EventNotifier::Read); - sigEvent_->activated.connect(this, &ProcessManager::sighandler); - - self_ = this; -} - -ProcessManager::~ProcessManager() -{ - sigaction(SIGCHLD, &oldsa_, NULL); - - delete sigEvent_; - - self_ = nullptr; -} - -/** - * \brief Retrieve the Process manager instance - * - * The ProcessManager is constructed by the CameraManager. This function shall - * be used to retrieve the single instance of the manager. - * - * \return The Process manager instance - */ -ProcessManager *ProcessManager::instance() -{ - return self_; -} - -/** - * \brief Retrieve the Process manager's write pipe - * - * This function is meant only to be used by the static signal handler. - * - * \return Pipe for writing - */ -int ProcessManager::writePipe() const -{ - return pipe_[1].get(); -} - -/** - * \brief Retrive the old signal action data - * - * This function is meant only to be used by the static signal handler. - * - * \return The old signal action data - */ -const struct sigaction &ProcessManager::oldsa() const -{ - return oldsa_; -} - /** * \class Process * \brief Process object @@ -290,8 +142,6 @@ int Process::start(const std::string &path, Span args, Span fds) { - int ret; - if (pid_ > 0) return -EBUSY; @@ -300,20 +150,29 @@ int Process::start(const std::string &path, return -EINVAL; } - int childPid = fork(); - if (childPid == -1) { - ret = -errno; + clone_args cargs = {}; + int pidfd; + + cargs.flags = CLONE_PIDFD | CLONE_NEWUSER | CLONE_NEWNET; + cargs.pidfd = reinterpret_cast(&pidfd); + cargs.exit_signal = SIGCHLD; + + long childPid = syscall(SYS_clone3, &cargs, sizeof(cargs)); + if (childPid < 0) { + int ret = -errno; LOG(Process, Error) << "Failed to fork: " << strerror(-ret); return ret; - } else if (childPid) { + } + + if (childPid) { pid_ = childPid; - ProcessManager::instance()->registerProcess(this); + pidfd_ = UniqueFD(pidfd); + pidfdNotify_ = std::make_unique(pidfd_.get(), EventNotifier::Type::Read); + pidfdNotify_->activated.connect(this, &Process::onPidfdNotify); - return 0; + LOG(Process, Debug) << this << "[" << childPid << ':' << pidfd << "]" + << " forked"; } else { - if (isolate()) - _exit(EXIT_FAILURE); - std::vector v(fds.begin(), fds.end()); v.push_back(STDERR_FILENO); closeAllFdsExcept(std::move(v)); @@ -346,35 +205,44 @@ int Process::start(const std::string &path, _exit(EXIT_FAILURE); } -} - -int Process::isolate() -{ - int ret = unshare(CLONE_NEWUSER | CLONE_NEWNET); - if (ret) { - ret = -errno; - LOG(Process, Error) << "Failed to unshare execution context: " - << strerror(-ret); - return ret; - } return 0; } -/** - * \brief SIGCHLD handler - * \param[in] wstatus The status as output by waitpid() - * - * This function is called when the process associated with Process terminates. - * It emits the Process::finished signal. - */ -void Process::died(int wstatus) +void Process::onPidfdNotify() { - pid_ = -1; - exitStatus_ = WIFEXITED(wstatus) ? NormalExit : SignalExit; - exitCode_ = exitStatus_ == NormalExit ? WEXITSTATUS(wstatus) : -1; + auto pidfdNotify = std::exchange(pidfdNotify_, {}); + auto pidfd = std::exchange(pidfd_, {}); + auto pid = std::exchange(pid_, -1); + + ASSERT(pidfdNotify); + ASSERT(pidfd.isValid()); + ASSERT(pid > 0); + + siginfo_t info; + + /* + * `static_cast` is needed because `P_PIDFD` is not defined in `sys/wait.h` if the C standard library + * is old enough. So `P_PIDFD` is taken from `linux/wait.h`, where it is just an integer #define. + */ + if (waitid(static_cast(P_PIDFD), pidfd.get(), &info, WNOHANG | WEXITED) >= 0) { + ASSERT(info.si_pid == pid); - finished.emit(exitStatus_, exitCode_); + LOG(Process, Debug) + << this << "[" << pid << ':' << pidfd.get() << "]" + << " code: " << info.si_code + << " status: " << info.si_status; + + exitStatus_ = info.si_code == CLD_EXITED ? Process::NormalExit : Process::SignalExit; + exitCode_ = info.si_code == CLD_EXITED ? info.si_status : -1; + + finished.emit(exitStatus_, exitCode_); + } else { + int err = errno; + LOG(Process, Warning) + << this << "[" << pid << ":" << pidfd.get() << "]" + << " waitid() failed: " << strerror(err); + } } /** @@ -412,8 +280,8 @@ void Process::died(int wstatus) */ void Process::kill() { - if (pid_ > 0) - ::kill(pid_, SIGKILL); + if (pidfd_.isValid()) + syscall(SYS_pidfd_send_signal, pidfd_.get(), SIGKILL, nullptr, 0); } } /* namespace libcamera */ diff --git a/test/ipc/unixsocket_ipc.cpp b/test/ipc/unixsocket_ipc.cpp index df7d9c2b4..f3e3c09ef 100644 --- a/test/ipc/unixsocket_ipc.cpp +++ b/test/ipc/unixsocket_ipc.cpp @@ -209,8 +209,6 @@ protected: } private: - ProcessManager processManager_; - unique_ptr ipc_; }; diff --git a/test/log/log_process.cpp b/test/log/log_process.cpp index 9609e23d5..367b78803 100644 --- a/test/log/log_process.cpp +++ b/test/log/log_process.cpp @@ -137,8 +137,6 @@ private: exitCode_ = exitCode; } - ProcessManager processManager_; - Process proc_; Process::ExitStatus exitStatus_ = Process::NotExited; string logPath_; diff --git a/test/process/process_test.cpp b/test/process/process_test.cpp index a88d8fef1..fe76666e6 100644 --- a/test/process/process_test.cpp +++ b/test/process/process_test.cpp @@ -86,8 +86,6 @@ private: exitCode_ = exitCode; } - ProcessManager processManager_; - Process proc_; enum Process::ExitStatus exitStatus_; int exitCode_;