From patchwork Thu Apr 24 11:21:54 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: 23243 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 C0945C327D for ; Thu, 24 Apr 2025 11:22:15 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 1D92768AD1; Thu, 24 Apr 2025 13:22:10 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="CriwA/8N"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 08B6168AC7 for ; Thu, 24 Apr 2025 13:22:07 +0200 (CEST) Received: from pb-laptop.local (185.221.143.16.nat.pool.zt.hu [185.221.143.16]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id CF5A216A; Thu, 24 Apr 2025 13:22:04 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1745493725; bh=/i8v1siOOqDzDe2JqXG0KEGLYIr+ubhZNLVjfELyg30=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=CriwA/8NkbA9Zyb/9ZzjA4pOLahpiBSSnmCTs34wMyc77WdIi4ONkhZVSTmFQ1sOT 9HPXvpuTyjzxLiGzLIFY1rBTCKA4NEshlE1kYc/yQ4TB+YTJSPBIq02xc3MO1vRQ58 V852D91G46hI4/aZOoT+h6jiJWovr33EJlIbhBlQ= From: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= To: libcamera-devel@lists.libcamera.org Cc: Kieran Bingham , Laurent Pinchart Subject: [RFC PATCH v4 1/9] libcamera: process: Disable copy/move Date: Thu, 24 Apr 2025 13:21:54 +0200 Message-ID: <20250424112203.445351-2-barnabas.pocze@ideasonboard.com> X-Mailer: git-send-email 2.49.0 In-Reply-To: <20250424112203.445351-1-barnabas.pocze@ideasonboard.com> References: <20250424112203.445351-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" A `Process` object has address identity because a pointer to it is stored inside the `ProcessManager`. However, copy/move special methods are still generated by the compiler. So disable them to avoid potential issues and confusion. Signed-off-by: Barnabás Pőcze Reviewed-by: Kieran Bingham Reviewed-by: Laurent Pinchart --- include/libcamera/internal/process.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/include/libcamera/internal/process.h b/include/libcamera/internal/process.h index b1d07a5a5..6c34aef2f 100644 --- a/include/libcamera/internal/process.h +++ b/include/libcamera/internal/process.h @@ -11,6 +11,7 @@ #include #include +#include #include #include @@ -42,6 +43,8 @@ public: Signal finished; private: + LIBCAMERA_DISABLE_COPY_AND_MOVE(Process) + void closeAllFdsExcept(const std::vector &fds); int isolate(); void died(int wstatus); From patchwork Thu Apr 24 11:21:56 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: 23245 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 A8A4DC331E for ; Thu, 24 Apr 2025 11:22:20 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 0425368ADC; Thu, 24 Apr 2025 13:22:14 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="MyJ9SB/k"; 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 98B6168AC5 for ; Thu, 24 Apr 2025 13:22:07 +0200 (CEST) Received: from pb-laptop.local (185.221.143.16.nat.pool.zt.hu [185.221.143.16]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 8480F1230; Thu, 24 Apr 2025 13:22:05 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1745493725; bh=fGC17eG1S/Jqp91DikygXMfnrbPJeAFxslQBqhJeatE=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=MyJ9SB/ksKBWltDXpDa5GO/SaKqvqtwFPGYG35q3Grw2vZvI1zCa0X6Oo6XHBPTO4 Pl3M7OQxv40Quqzv9UWm5gd3+ldcS2pQYwoGsFQdCwhMmPPtS/S1JBfxkUSyaP9TA+ xRXWwVInkFPa49rH9TjraFY0Rw3UxN9sDoxwjQ+o= From: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= To: libcamera-devel@lists.libcamera.org Cc: Kieran Bingham , Laurent Pinchart Subject: [RFC PATCH v4 3/9] libcamera: process: Use `pid_` member to decide if running Date: Thu, 24 Apr 2025 13:21:56 +0200 Message-ID: <20250424112203.445351-4-barnabas.pocze@ideasonboard.com> X-Mailer: git-send-email 2.49.0 In-Reply-To: <20250424112203.445351-1-barnabas.pocze@ideasonboard.com> References: <20250424112203.445351-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 0990b4bdd..c22c9af5d 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 0; int childPid = fork(); @@ -252,8 +252,6 @@ int Process::start(const std::string &path, pid_ = childPid; ProcessManager::instance()->registerProcess(this); - running_ = true; - return 0; } else { if (isolate()) @@ -327,7 +325,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 Thu Apr 24 11:21:57 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: 23246 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 18CEEC331F for ; Thu, 24 Apr 2025 11:22:21 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 37F2568AD3; Thu, 24 Apr 2025 13:22:16 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="Xd/K6u7h"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id F0DB868ACE for ; Thu, 24 Apr 2025 13:22:07 +0200 (CEST) Received: from pb-laptop.local (185.221.143.16.nat.pool.zt.hu [185.221.143.16]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id D8B7A1440; Thu, 24 Apr 2025 13:22:05 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1745493726; bh=Qz9cUuFaDliK3CR4EEBKiEQbQrPsPaP/+AEhQUc9DRE=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=Xd/K6u7heU03in0Cs2iHNJnr/iDQ77qw8TMfSO7nDxZXRYtCGz05AmBtrSLSD3UDg kaucWfV5r05WXY5dgbECVMfqS8HCmtT8+nvmP4U2WsviVGoLTdoZvVl//va+85U+Dr xfyIGfaCrJn8fnddsrU/2ESlTDcQ7iMmtUa2yj+M= From: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= To: libcamera-devel@lists.libcamera.org Cc: Kieran Bingham , Laurent Pinchart Subject: [RFC PATCH v4 4/9] libcamera: process: Return error if already running Date: Thu, 24 Apr 2025 13:21:57 +0200 Message-ID: <20250424112203.445351-5-barnabas.pocze@ideasonboard.com> X-Mailer: git-send-email 2.49.0 In-Reply-To: <20250424112203.445351-1-barnabas.pocze@ideasonboard.com> References: <20250424112203.445351-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" Returning 0 when a running process is already managed can be confusing since the parameters might be completely different, causing the caller to mistakenly assume that the program it specified has been started. Signed-off-by: Barnabás Pőcze Reviewed-by: Kieran Bingham Reviewed-by: Laurent Pinchart --- src/libcamera/process.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libcamera/process.cpp b/src/libcamera/process.cpp index c22c9af5d..ca9733d77 100644 --- a/src/libcamera/process.cpp +++ b/src/libcamera/process.cpp @@ -241,7 +241,7 @@ int Process::start(const std::string &path, int ret; if (pid_ > 0) - return 0; + return -EBUSY; int childPid = fork(); if (childPid == -1) { From patchwork Thu Apr 24 11:21:58 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: 23247 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 06CBDC3320 for ; Thu, 24 Apr 2025 11:22:23 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 97BE368B27; Thu, 24 Apr 2025 13:22:17 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="r48uVZU/"; 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 537C668AC7 for ; Thu, 24 Apr 2025 13:22:08 +0200 (CEST) Received: from pb-laptop.local (185.221.143.16.nat.pool.zt.hu [185.221.143.16]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 3F3C91666; Thu, 24 Apr 2025 13:22:06 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1745493726; bh=6SAo2VztWOdl3IaEm7dGNgRt6EEEbGm3QhaZW53aXf4=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=r48uVZU/R5j2T6SsKnmnc9g3tiICHHqYkGHGcl5agl6rP54oZUE8+43zc6+LYTyl8 7LMIQxpS7tdEMA6pMXDqaJUFQ63SBkhx2B0ZlTbP/uC6md8eH7tlS85mLmbl/yTZio /6Sl1afO7d3mt1YRoTlOQfER2MtETx7EiRSAqWWY= From: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= To: libcamera-devel@lists.libcamera.org Cc: Kieran Bingham , Laurent Pinchart Subject: [RFC PATCH v4 5/9] libcamera: process: Ensure that file descriptors are nonnegative Date: Thu, 24 Apr 2025 13:21:58 +0200 Message-ID: <20250424112203.445351-6-barnabas.pocze@ideasonboard.com> X-Mailer: git-send-email 2.49.0 In-Reply-To: <20250424112203.445351-1-barnabas.pocze@ideasonboard.com> References: <20250424112203.445351-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" Return `-EINVAL` from `Process::start()` if any of the file descriptors are negative as those most likely signal some kind of issue such as missed error checking. Signed-off-by: Barnabás Pőcze Reviewed-by: Kieran Bingham Reviewed-by: Laurent Pinchart --- src/libcamera/process.cpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/libcamera/process.cpp b/src/libcamera/process.cpp index ca9733d77..1bc7a7f9a 100644 --- a/src/libcamera/process.cpp +++ b/src/libcamera/process.cpp @@ -243,6 +243,11 @@ int Process::start(const std::string &path, if (pid_ > 0) return -EBUSY; + for (int fd : fds) { + if (fd < 0) + return -EINVAL; + } + int childPid = fork(); if (childPid == -1) { ret = -errno; @@ -282,6 +287,8 @@ void Process::closeAllFdsExcept(const std::vector &fds) std::vector v(fds); sort(v.begin(), v.end()); + ASSERT(v.empty() || v.front() >= 0); + DIR *dir = opendir("/proc/self/fd"); if (!dir) return; From patchwork Thu Apr 24 11:21:59 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: 23248 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 A317BC3321 for ; Thu, 24 Apr 2025 11:22:25 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 2632468B2B; Thu, 24 Apr 2025 13:22:19 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="VEffDEwO"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 9E96168AD3 for ; Thu, 24 Apr 2025 13:22:08 +0200 (CEST) Received: from pb-laptop.local (185.221.143.16.nat.pool.zt.hu [185.221.143.16]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 95C9D18E5 for ; Thu, 24 Apr 2025 13:22:06 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1745493726; bh=MZl7tDpA6SRqfheZTd2QqlxmlX3SfZjKHJ+upIArxSs=; h=From:To:Subject:Date:In-Reply-To:References:From; b=VEffDEwODvyvrj8FlJSsvoMLawVf7OYLyrbuioc4G0z3AXl9n3gCRVDzi6OH7W05c 1z0/F+ctFe0SWsgbI04LGcWRXngXGqXj892IEtOFilp4MrXjuF1ZvROjYfJHkAZKgc UtmrkUgKbSsRhxBTzXQ6fdCFLXA4DI922H2flhgM= From: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= To: libcamera-devel@lists.libcamera.org Subject: [RFC PATCH v4 6/9] libcamera: process: Use span instead of vector Date: Thu, 24 Apr 2025 13:21:59 +0200 Message-ID: <20250424112203.445351-7-barnabas.pocze@ideasonboard.com> X-Mailer: git-send-email 2.49.0 In-Reply-To: <20250424112203.445351-1-barnabas.pocze@ideasonboard.com> References: <20250424112203.445351-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 `libcamera::Span` whenever applicable. Signed-off-by: Barnabás Pőcze --- include/libcamera/internal/process.h | 8 ++++---- src/libcamera/ipc_pipe_unixsocket.cpp | 9 +++------ src/libcamera/process.cpp | 8 ++++---- test/process/process_test.cpp | 5 ++--- 4 files changed, 13 insertions(+), 17 deletions(-) diff --git a/include/libcamera/internal/process.h b/include/libcamera/internal/process.h index 891e78947..b8eb7e043 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_; } @@ -45,7 +45,7 @@ public: private: LIBCAMERA_DISABLE_COPY_AND_MOVE(Process) - void closeAllFdsExcept(const std::vector &fds); + void closeAllFdsExcept(Span fds); int isolate(); void died(int wstatus); 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 1bc7a7f9a..99bc3ad36 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; @@ -282,9 +282,9 @@ int Process::start(const std::string &path, } } -void Process::closeAllFdsExcept(const std::vector &fds) +void Process::closeAllFdsExcept(Span fds) { - std::vector v(fds); + std::vector v(fds.begin(), fds.end()); sort(v.begin(), v.end()); ASSERT(v.empty() || v.front() >= 0); 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 Thu Apr 24 11:22:00 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: 23249 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 003FFC3322 for ; Thu, 24 Apr 2025 11:22:26 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 3DB7A68AD1; Thu, 24 Apr 2025 13:22:21 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="I9xSrmvo"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id D162168AD5 for ; Thu, 24 Apr 2025 13:22:08 +0200 (CEST) Received: from pb-laptop.local (185.221.143.16.nat.pool.zt.hu [185.221.143.16]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id D59BE16A for ; Thu, 24 Apr 2025 13:22:06 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1745493726; bh=X2o2EEnbixf3rrcrZfDkTDGdmSlEUbfzYv236RR2Je0=; h=From:To:Subject:Date:In-Reply-To:References:From; b=I9xSrmvon3Q0Vqm89bsWvuNGJw8HPjQUsbKobazSJCDlxk8oRGil6kt+OVI4JvGsG k0iZTeyNdxdoTFUpmzQ3yOQ8VqOTQ8Mo9EqfKbzYP7fczFiPawDKU07mDESi3kWubu kBni70pejWlgm9geUsGHmA77AfVEpjmb36l7pZnY= From: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= To: libcamera-devel@lists.libcamera.org Subject: [RFC PATCH v4 7/9] libcamera: process: Move `closeAllFdsExcept()` Date: Thu, 24 Apr 2025 13:22:00 +0200 Message-ID: <20250424112203.445351-8-barnabas.pocze@ideasonboard.com> X-Mailer: git-send-email 2.49.0 In-Reply-To: <20250424112203.445351-1-barnabas.pocze@ideasonboard.com> References: <20250424112203.445351-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 no needed "outside" and does not depend on any part of the `Process` class. Signed-off-by: Barnabás Pőcze --- include/libcamera/internal/process.h | 1 - src/libcamera/process.cpp | 56 ++++++++++++++-------------- 2 files changed, 28 insertions(+), 29 deletions(-) diff --git a/include/libcamera/internal/process.h b/include/libcamera/internal/process.h index b8eb7e043..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(Span fds); int isolate(); void died(int wstatus); diff --git a/src/libcamera/process.cpp b/src/libcamera/process.cpp index 99bc3ad36..369fdb12d 100644 --- a/src/libcamera/process.cpp +++ b/src/libcamera/process.cpp @@ -63,6 +63,34 @@ void sigact(int signal, siginfo_t *info, void *ucontext) } } +void closeAllFdsExcept(Span fds) +{ + std::vector v(fds.begin(), fds.end()); + 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() @@ -282,34 +310,6 @@ int Process::start(const std::string &path, } } -void Process::closeAllFdsExcept(Span fds) -{ - std::vector v(fds.begin(), fds.end()); - 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 Thu Apr 24 11:22:01 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: 23250 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 54121C3323 for ; Thu, 24 Apr 2025 11:22:28 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id E277968B26; Thu, 24 Apr 2025 13:22:22 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="q/x6otVe"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 25A1268AD7 for ; Thu, 24 Apr 2025 13:22:09 +0200 (CEST) Received: from pb-laptop.local (185.221.143.16.nat.pool.zt.hu [185.221.143.16]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 20129F01; Thu, 24 Apr 2025 13:22:07 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1745493727; bh=yERMYMCMd6M3R7agseyITlcqSb9qdaTssWxyeDjLHNI=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=q/x6otVefyB2Wqr+DjISz7zD2SLilgydpjoYm1BUbZ7Cz9AxDJmP3wSlYAhnxufnt hV33rLlz2thybrqKOoWzBAF2GZKOZriH39ESc8EkEK3u2JrrmZTyMX5K9AU557NeF/ pUdzGg1lTxGT6K/8c20m9g9D6OZi9c74rAUFpICw= From: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= To: libcamera-devel@lists.libcamera.org Cc: Laurent Pinchart Subject: [RFC PATCH v4 8/9] libcamera: process: Use `close_range()` when available Date: Thu, 24 Apr 2025 13:22:01 +0200 Message-ID: <20250424112203.445351-9-barnabas.pocze@ideasonboard.com> X-Mailer: git-send-email 2.49.0 In-Reply-To: <20250424112203.445351-1-barnabas.pocze@ideasonboard.com> References: <20250424112203.445351-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 --- meson.build | 4 ++++ src/libcamera/process.cpp | 25 +++++++++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/meson.build b/meson.build index e9d6187b3..1525bdf4a 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 369fdb12d..42b56374d 100644 --- a/src/libcamera/process.cpp +++ b/src/libcamera/process.cpp @@ -70,6 +70,31 @@ void closeAllFdsExcept(Span fds) 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 Thu Apr 24 11:22:02 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: 23251 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 C1933C3324 for ; Thu, 24 Apr 2025 11:22:29 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 7601168AD7; Thu, 24 Apr 2025 13:22:24 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="eC/KueJZ"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 70F1668AD8 for ; Thu, 24 Apr 2025 13:22:09 +0200 (CEST) Received: from pb-laptop.local (185.221.143.16.nat.pool.zt.hu [185.221.143.16]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 685C316A for ; Thu, 24 Apr 2025 13:22:07 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1745493727; bh=dNQ7Uo2BUTUD18/Av4Yqlj30bUMQ3mo9KsMR0e/491g=; h=From:To:Subject:Date:In-Reply-To:References:From; b=eC/KueJZrECT1QUtqY7ui33KJUIuzv5iSbjzRkgt1/ZzIp/VkfU2uM1q3W6eh4Av5 zCUCitMyOOTCRl/uu3fQbxS83aXINclQa5xCJVygnT3ekL3ziJVXGC3xQl7EC0/12T /FzayGJihu6+BfVljvpjDXx9c4kYiELeD5fJ7yDk= From: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= To: libcamera-devel@lists.libcamera.org Subject: [RFC PATCH v4 9/9] libcamera: process: Remove `ProcessManager` singleton Date: Thu, 24 Apr 2025 13:22:02 +0200 Message-ID: <20250424112203.445351-10-barnabas.pocze@ideasonboard.com> X-Mailer: git-send-email 2.49.0 In-Reply-To: <20250424112203.445351-1-barnabas.pocze@ideasonboard.com> References: <20250424112203.445351-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 --- 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 1525bdf4a..83c4b87de 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 42b56374d..124677038 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(Span fds) { std::vector v(fds.begin(), fds.end()); @@ -118,129 +93,6 @@ void closeAllFdsExcept(Span fds) } /* 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 @@ -291,8 +143,6 @@ int Process::start(const std::string &path, Span args, Span fds) { - int ret; - if (pid_ > 0) return -EBUSY; @@ -301,20 +151,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_CLEAR_SIGHAND | 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); - closeAllFdsExcept(fds); const char *file = utils::secure_getenv("LIBCAMERA_LOG_FILE"); @@ -333,35 +192,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); + } } /** @@ -399,8 +267,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_;