From patchwork Thu Apr 24 11:40:55 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: 23253 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 CAAFAC327D for ; Thu, 24 Apr 2025 11:41:14 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 90CA968AD9; Thu, 24 Apr 2025 13:41:11 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="JXEipIl4"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 7DCF168ACD for ; Thu, 24 Apr 2025 13:41: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 2CD601230; Thu, 24 Apr 2025 13:41:05 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1745494865; bh=/i8v1siOOqDzDe2JqXG0KEGLYIr+ubhZNLVjfELyg30=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=JXEipIl4FKSnlzpfaMp3dVJa5FOnZK3zCPB8vmKwxerNn7tRYExKm0uEQUDBmdt+4 7Q8B0W7QXenutq8oE7ybmNHXEdW/P6uCd/hAEYwFXTY+gs5idSpAhTQ6tOKGAp4ak/ ZeULYtRFH61Vmc8MTMf4kNH/odfoKzOnjo4naxwk= From: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= To: libcamera-devel@lists.libcamera.org Cc: Kieran Bingham , Laurent Pinchart Subject: [RFC PATCH v5 1/9] libcamera: process: Disable copy/move Date: Thu, 24 Apr 2025 13:40:55 +0200 Message-ID: <20250424114103.451395-2-barnabas.pocze@ideasonboard.com> X-Mailer: git-send-email 2.49.0 In-Reply-To: <20250424114103.451395-1-barnabas.pocze@ideasonboard.com> References: <20250424114103.451395-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:40: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: 23254 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 B0CE4C32A2 for ; Thu, 24 Apr 2025 11:41:16 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 8EFCD68ACE; Thu, 24 Apr 2025 13:41:12 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="j2HMYSBX"; 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 C25B668ACE for ; Thu, 24 Apr 2025 13:41: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 811B91440; Thu, 24 Apr 2025 13:41:05 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1745494865; bh=v+d5do+BDii4mzQmG2m0z5d+XRpJdYj73QV9LWHAWRs=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=j2HMYSBXkK7GhMPUKx9j1VOGpJepOgnR6rYFkJhP+OJgeI/MLvUXgZEuQRT1PyT2R C7GFty4VapFcPdkz/rE4vFSqpfEIcOVswd9ptPSVtMRREJH44AxQbbzW5iBF7/QWwK azcWWaE43J2zNp7Dz1XK+mSqD+odQmDnwBKYFMGs= From: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= To: libcamera-devel@lists.libcamera.org Cc: Kieran Bingham , Laurent Pinchart Subject: [RFC PATCH v5 2/9] libcamera: process: Misc. cleanup around `execv()` Date: Thu, 24 Apr 2025 13:40:56 +0200 Message-ID: <20250424114103.451395-3-barnabas.pocze@ideasonboard.com> X-Mailer: git-send-email 2.49.0 In-Reply-To: <20250424114103.451395-1-barnabas.pocze@ideasonboard.com> References: <20250424114103.451395-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" Firstly, get the number of arguments first, and use that to determine the size of the allocation instead of retrieving it twice. Secondly, use `const_cast` instead of a C-style cast when calling `execv()`. Third, use `size_t` to match the type of `args.size()`. Signed-off-by: Barnabás Pőcze Reviewed-by: Kieran Bingham Reviewed-by: Laurent Pinchart --- src/libcamera/process.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/libcamera/process.cpp b/src/libcamera/process.cpp index 68fad3270..0990b4bdd 100644 --- a/src/libcamera/process.cpp +++ b/src/libcamera/process.cpp @@ -265,14 +265,15 @@ int Process::start(const std::string &path, if (file && strcmp(file, "syslog")) unsetenv("LIBCAMERA_LOG_FILE"); - const char **argv = new const char *[args.size() + 2]; - unsigned int len = args.size(); + const size_t len = args.size(); + auto argv = std::make_unique(len + 2); + argv[0] = path.c_str(); - for (unsigned int i = 0; i < len; i++) + for (size_t i = 0; i < len; i++) argv[i + 1] = args[i].c_str(); argv[len + 1] = nullptr; - execv(path.c_str(), (char **)argv); + execv(path.c_str(), const_cast(argv.get())); exit(EXIT_FAILURE); } From patchwork Thu Apr 24 11:40: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: 23255 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 1EE99C331E for ; Thu, 24 Apr 2025 11:41:18 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 4F95F68B28; Thu, 24 Apr 2025 13:41:13 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="NYeVj0+O"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id F0B4F68AD0 for ; Thu, 24 Apr 2025 13:41: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 D39AEF01; Thu, 24 Apr 2025 13:41:05 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1745494866; bh=fGC17eG1S/Jqp91DikygXMfnrbPJeAFxslQBqhJeatE=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=NYeVj0+Oa/Tklvs73k5kvO279rJUDKyv5NFOmYiKIl5qEfCeadnvEyJtIeAaPDTwX W7JEcC5Haklv/rM8cS58GlKcRRfhVxF29FOkzDwhthXE2GmHh9Gl2GMiKL4QGiZmR5 FGaesYCk+vP6bYSZ1vbsJXj7q9VR3xkuidkV+1UY= From: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= To: libcamera-devel@lists.libcamera.org Cc: Kieran Bingham , Laurent Pinchart Subject: [RFC PATCH v5 3/9] libcamera: process: Use `pid_` member to decide if running Date: Thu, 24 Apr 2025 13:40:57 +0200 Message-ID: <20250424114103.451395-4-barnabas.pocze@ideasonboard.com> X-Mailer: git-send-email 2.49.0 In-Reply-To: <20250424114103.451395-1-barnabas.pocze@ideasonboard.com> References: <20250424114103.451395-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:40: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: 23256 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 75E12C327D for ; Thu, 24 Apr 2025 11:41:19 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 8CF7268AD5; Thu, 24 Apr 2025 13:41:14 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="qE82I3ol"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 4EB7B68ACC for ; Thu, 24 Apr 2025 13:41: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 3674F1666; Thu, 24 Apr 2025 13:41:06 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1745494866; bh=Qz9cUuFaDliK3CR4EEBKiEQbQrPsPaP/+AEhQUc9DRE=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=qE82I3olg4Er13shwbs8T1XAI7zaZeKcHq4ZveIj/ljZwpEukCRsaf3QUNeEJeCBa O422XIZ5xrWqk/tJQGw+ivpD6JYCHdvj6Hqqy6zai2RPlkgM2ihViy2KhZOk2K3CmQ Zq+U7F49cels9R9OsTRZYoo++GZ2sCw/5dyr20kA= From: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= To: libcamera-devel@lists.libcamera.org Cc: Kieran Bingham , Laurent Pinchart Subject: [RFC PATCH v5 4/9] libcamera: process: Return error if already running Date: Thu, 24 Apr 2025 13:40:58 +0200 Message-ID: <20250424114103.451395-5-barnabas.pocze@ideasonboard.com> X-Mailer: git-send-email 2.49.0 In-Reply-To: <20250424114103.451395-1-barnabas.pocze@ideasonboard.com> References: <20250424114103.451395-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:40: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: 23257 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 851C8C331F for ; Thu, 24 Apr 2025 11:41:20 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id BE83368B24; Thu, 24 Apr 2025 13:41:15 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="QkbU2kfD"; 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 A2A8B68AD3 for ; Thu, 24 Apr 2025 13:41: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 89E5318E5; Thu, 24 Apr 2025 13:41:06 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1745494866; bh=6SAo2VztWOdl3IaEm7dGNgRt6EEEbGm3QhaZW53aXf4=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=QkbU2kfDj+QfPb9WRC5gR/nP0m+GvYq1a7O+hkZOifnFgcUd728ClJ8hjfBfqDpgh JZxi1XivkRjnc9oXTnro/hTBNbcbjh0v80UTNCFDxgxfPcIKM0SGdPu1aTJpJZVPaU 64vrvMkc8izwWqMnz86Wi/+0z25wAjoGqNaGbdBI= From: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= To: libcamera-devel@lists.libcamera.org Cc: Kieran Bingham , Laurent Pinchart Subject: [RFC PATCH v5 5/9] libcamera: process: Ensure that file descriptors are nonnegative Date: Thu, 24 Apr 2025 13:40:59 +0200 Message-ID: <20250424114103.451395-6-barnabas.pocze@ideasonboard.com> X-Mailer: git-send-email 2.49.0 In-Reply-To: <20250424114103.451395-1-barnabas.pocze@ideasonboard.com> References: <20250424114103.451395-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:41: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: 23258 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 49F4CC3320 for ; Thu, 24 Apr 2025 11:41:21 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 56DDF68B25; Thu, 24 Apr 2025 13:41:16 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="F4c1fWQO"; 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 52AFB68AD3 for ; Thu, 24 Apr 2025 13:41: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 DC32219BF; Thu, 24 Apr 2025 13:41:06 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1745494867; bh=sGQU65lJ1MeDo8vv5WS07r8IYOXw2o/eIvT6X28zCLM=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=F4c1fWQOnQuCLAgDBjy9/PkQ885VZgqR1v4o/KpHWMfDffD5yMJ1/bem0D6NhghgT UeoOVO/cGSI+lMy3tpTIsGcufbPhWpAU6SZr6x1S0OQ3rygKHDG2XqF+RnjcsG3/tJ 3FcVKHJkjQj3tIlT3j1golhSGS6JRaauhE7A/LOM= From: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= To: libcamera-devel@lists.libcamera.org Cc: Kieran Bingham , Laurent Pinchart Subject: [RFC PATCH v5 6/9] libcamera: process: Use span instead of vector Date: Thu, 24 Apr 2025 13:41:00 +0200 Message-ID: <20250424114103.451395-7-barnabas.pocze@ideasonboard.com> X-Mailer: git-send-email 2.49.0 In-Reply-To: <20250424114103.451395-1-barnabas.pocze@ideasonboard.com> References: <20250424114103.451395-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 Reviewed-by: Kieran Bingham Reviewed-by: Laurent Pinchart --- 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:41: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: 23259 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 EEA10C3321 for ; Thu, 24 Apr 2025 11:41:21 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 1502368AD2; Thu, 24 Apr 2025 13:41:17 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="JT5mr0SB"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 294AF68ADB for ; Thu, 24 Apr 2025 13:41:10 +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 F096019EB; Thu, 24 Apr 2025 13:41:07 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1745494868; bh=pHOL+k+Dy/DcIWAJ0Djp8YpzXaUs4tOTqsrd2bdl+fA=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=JT5mr0SBCSHsEHRF4X5fd4y37nc5rf4YXUN54sMoy8dDMHjFooR3c7gC/xjX5ZzQt zj0iDrtyBvRPqa9ulDMPSxLja5+6mIoNHWrGPN6UVsX/TwgGU60KP1CNJfSYRqiRMV PAZzvVGWpaJpohaYnlEfTcvLmY9XHMVR9g+/SD/c= From: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= To: libcamera-devel@lists.libcamera.org Cc: Laurent Pinchart Subject: [RFC PATCH v5 7/9] libcamera: process: Move `closeAllFdsExcept()` Date: Thu, 24 Apr 2025 13:41:01 +0200 Message-ID: <20250424114103.451395-8-barnabas.pocze@ideasonboard.com> X-Mailer: git-send-email 2.49.0 In-Reply-To: <20250424114103.451395-1-barnabas.pocze@ideasonboard.com> References: <20250424114103.451395-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 Reviewed-by: Laurent Pinchart --- 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:41: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: 23260 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 7054EC32A2 for ; Thu, 24 Apr 2025 11:41:22 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 558D268B32; Thu, 24 Apr 2025 13:41:18 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="ELi6HAki"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id AB9E668AD1 for ; Thu, 24 Apr 2025 13:41:10 +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 7434D1A99; Thu, 24 Apr 2025 13:41:08 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1745494868; bh=aOOx0AFCP9uaRXubSVrh4PGb0/Mp1a/g1L8iuB7sCOc=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=ELi6HAkiGz6wuReCFOKyFaKqcwXEbtrC5pLXUktODmEmzMkzWRWBKyAph5xddByjs A2kuLLK8Hw0Ry48DBsT7GSzHm7GDVgJr72RQbPZ1bQhVT1p2cW0ARUXVN4l/5Pr2CC cm00kZBiRPtLRsMywfZR0lMMXSu7FcD+21Xvco8U= From: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= To: libcamera-devel@lists.libcamera.org Cc: Laurent Pinchart Subject: [RFC PATCH v5 8/9] libcamera: process: Use `close_range()` when available Date: Thu, 24 Apr 2025 13:41:02 +0200 Message-ID: <20250424114103.451395-9-barnabas.pocze@ideasonboard.com> X-Mailer: git-send-email 2.49.0 In-Reply-To: <20250424114103.451395-1-barnabas.pocze@ideasonboard.com> References: <20250424114103.451395-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 9ba5e2ca9..ae13727eb 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:41:03 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: 23261 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 DD8E3C3322 for ; Thu, 24 Apr 2025 11:41:22 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 74EBE68B28; Thu, 24 Apr 2025 13:41:19 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="nwmRyDdG"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 5DBF468AD2 for ; Thu, 24 Apr 2025 13:41:11 +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 0F1091AD6; Thu, 24 Apr 2025 13:41:09 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1745494869; bh=fTHTxaMZYeXa20Ma4qLGZrU3f/OcK3HoEZ9oJrIf9kM=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=nwmRyDdGP2B+Mfj29xgEPJmcadba0BApXzAEMzSm03lNZZiBpKKAmhAQSuaZP4xGP MVCqED3TFe9KzTJparQ9qrMITiy007c4D2dwTxvJC96iDtjlKYB2FZ6fzSdNT3pLZ7 vYaUS7o0QPXSXSlcoM6JMAh69kIqc5ZYLq5KUXtI= From: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= To: libcamera-devel@lists.libcamera.org Cc: Laurent Pinchart Subject: [RFC PATCH v5 9/9] libcamera: process: Remove `ProcessManager` singleton Date: Thu, 24 Apr 2025 13:41:03 +0200 Message-ID: <20250424114103.451395-10-barnabas.pocze@ideasonboard.com> X-Mailer: git-send-email 2.49.0 In-Reply-To: <20250424114103.451395-1-barnabas.pocze@ideasonboard.com> References: <20250424114103.451395-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 | 245 +++++--------------- test/ipc/unixsocket_ipc.cpp | 2 - test/log/log_process.cpp | 2 - test/process/process_test.cpp | 2 - 7 files changed, 60 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 ae13727eb..3c895ae9e 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..b01c78034 100644 --- a/src/libcamera/process.cpp +++ b/src/libcamera/process.cpp @@ -10,15 +10,18 @@ #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 +35,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 +92,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 +142,6 @@ int Process::start(const std::string &path, Span args, Span fds) { - int ret; - if (pid_ > 0) return -EBUSY; @@ -301,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_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 +191,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 +266,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_;