[libcamera-devel] [PATCH v4 3/8] libcamera: Add Process and ProcessManager classes
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Fri Jul 12 08:35:19 CEST 2019
Hi Paul,
Thank you for the patch.
On Fri, Jul 12, 2019 at 03:50:42AM +0900, Paul Elder wrote:
> Add a Process class to abstract a process, and a ProcessManager singleton
> to monitor and manage the processes.
>
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> ---
> Changes in v4:
> - rename enum ExitStatus members
> - use a running_ flag (instead of state_)
> - fix sigaction registration (on Process construction)
> - restore old signal handler (on Process destruction)
> - replace closefrom_except() with closeAllFdsExcept()
> - unsetenv LIBCAMERA_LOG_FILE on fork (before exec)
>
> Changes in v3:
> - add Process test
> - move ProcessManager header to process.cpp
> - make Process final
> - add a bunch of things for monitoring and signals on process
> termination
>
> New in v2
>
> src/libcamera/include/process.h | 55 +++++
> src/libcamera/meson.build | 3 +
> src/libcamera/process.cpp | 357 ++++++++++++++++++++++++++++++
> src/libcamera/process_manager.cpp | 0
> test/meson.build | 1 +
> test/process/meson.build | 12 +
> test/process/process_test.cpp | 100 +++++++++
> 7 files changed, 528 insertions(+)
> create mode 100644 src/libcamera/include/process.h
> create mode 100644 src/libcamera/process.cpp
> create mode 100644 src/libcamera/process_manager.cpp
> create mode 100644 test/process/meson.build
> create mode 100644 test/process/process_test.cpp
>
> diff --git a/src/libcamera/include/process.h b/src/libcamera/include/process.h
> new file mode 100644
> index 0000000..d322fce
> --- /dev/null
> +++ b/src/libcamera/include/process.h
> @@ -0,0 +1,55 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2019, Google Inc.
> + *
> + * process.h - Process object
> + */
> +#ifndef __LIBCAMERA_PROCESS_H__
> +#define __LIBCAMERA_PROCESS_H__
> +
> +#include <string>
> +#include <vector>
> +
> +#include <libcamera/event_notifier.h>
> +
> +namespace libcamera {
> +
> +class Process final
> +{
> +public:
> + enum ExitStatus {
> + NotExited,
> + NormalExit,
> + SignalExit,
> + };
> +
> + Process();
> + ~Process();
> +
> + int start(const std::string &path,
> + const std::vector<std::string> &args = std::vector<std::string>(),
> + const std::vector<int> &fds = std::vector<int>());
> +
> + ExitStatus exitStatus() const { return exitStatus_; }
> + int exitCode() const { return exitCode_; }
> +
> + void kill();
> +
> + Signal<Process *, enum ExitStatus, int> finished;
> +
> +private:
> + void closeAllFdsExcept(const std::vector<int> &fds);
> + int isolate();
> + void died(int wstatus);
> +
> + pid_t pid_;
> + bool running_;
> + enum ExitStatus exitStatus_;
> + int exitCode_;
> +
> + friend class ProcessManager;
> +};
> +
> +} /* namespace libcamera */
> +
> +#endif /* __LIBCAMERA_PROCESS_H__ */
> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
> index eda506b..01565c1 100644
> --- a/src/libcamera/meson.build
> +++ b/src/libcamera/meson.build
> @@ -21,6 +21,8 @@ libcamera_sources = files([
> 'message.cpp',
> 'object.cpp',
> 'pipeline_handler.cpp',
> + 'process.cpp',
> + 'process_manager.cpp',
> 'request.cpp',
> 'signal.cpp',
> 'stream.cpp',
> @@ -48,6 +50,7 @@ libcamera_headers = files([
> 'include/media_object.h',
> 'include/message.h',
> 'include/pipeline_handler.h',
> + 'include/process.h',
> 'include/thread.h',
> 'include/utils.h',
> 'include/v4l2_device.h',
> diff --git a/src/libcamera/process.cpp b/src/libcamera/process.cpp
> new file mode 100644
> index 0000000..736d59f
> --- /dev/null
> +++ b/src/libcamera/process.cpp
> @@ -0,0 +1,357 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2019, Google Inc.
> + *
> + * process.cpp - Process object
> + */
> +
> +#include "process.h"
> +
> +#include <algorithm>
> +#include <dirent.h>
> +#include <fcntl.h>
> +#include <iostream>
> +#include <list>
> +#include <signal.h>
> +#include <string.h>
> +#include <sys/socket.h>
> +#include <sys/types.h>
> +#include <sys/wait.h>
> +#include <unistd.h>
> +#include <vector>
> +
> +#include <libcamera/event_notifier.h>
> +
> +#include "log.h"
> +#include "utils.h"
> +
> +/**
> + * \file process.h
> + * \brief Process object
> + */
> +
> +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.
> + */
> +class ProcessManager
> +{
> +public:
> + void registerProcess(Process *proc);
> +
> + static ProcessManager *instance();
> +
> + int writePipe() const;
> +
> + const struct sigaction &oldsa() const;
> +
> +private:
> + void sighandler(EventNotifier *notifier);
> + ProcessManager();
> + ~ProcessManager();
> +
> + std::list<Process *> processes_;
> +
> + struct sigaction oldsa_;
> + EventNotifier *sigEvent_;
> + int pipe_[2];
> +};
> +
> +namespace {
> +
> +void sigact(int signal, siginfo_t *info, void *ucontext)
> +{
> + char data = 0;
> + write(ProcessManager::instance()->writePipe(), &data, sizeof(data));
> +
> + 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);
> + }
> +}
> +
> +} /* namespace */
> +
> +void ProcessManager::sighandler(EventNotifier *notifier)
> +{
> + char data;
> + read(pipe_[0], &data, sizeof(data));
> +
> + 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 method 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()
> +{
> + 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);
> +
> + pipe2(pipe_, O_CLOEXEC | O_DIRECT | O_NONBLOCK);
> + sigEvent_ = new EventNotifier(pipe_[0], EventNotifier::Read);
> + sigEvent_->activated.connect(this, &ProcessManager::sighandler);
> +}
> +
> +ProcessManager::~ProcessManager()
> +{
> + delete sigEvent_;
> + close(pipe_[0]);
> + close(pipe_[1]);
> + sigaction(SIGCHLD, &oldsa_, NULL);
> +}
> +
> +/**
> + * \brief Retrieve the Process manager instance
> + *
> + * The ProcessManager is a singleton and can't be constructed manually. This
> + * method shall instead be used to retrieve the single global instance of the
> + * manager.
> + *
> + * \return The Process manager instance
> + */
> +ProcessManager *ProcessManager::instance()
> +{
> + static ProcessManager processManager;
> + return &processManager;
> +}
> +
> +/**
> + * \brief Retrieve the Process manager's write pipe
> + *
> + * This method is meant only to be used by the static signal handler.
> + *
> + * \return Pipe for writing
> + */
> +int ProcessManager::writePipe() const
> +{
> + return pipe_[1];
> +}
> +
> +/**
> + * \brief Retrive the old signal action data
> + *
> + * This method 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
> + *
> + * The Process class models a process, and simplifies spawning new processes
> + * and monitoring the exiting of a process.
> + */
> +
> +/**
> + * \enum Process::ExitStatus
> + * \brief Exit status of process
> + * \var Process::NotExited
> + * The process hasn't exited yet
> + * \var Process::NormalExit
> + * The process exited normally, either via exit() or returning from main
> + * \var Process::SignalExit
> + * The process was terminated by a signal (this includes crashing)
> + */
> +
> +Process::Process()
> + : pid_(-1), running_(false), exitStatus_(NotExited), exitCode_(0)
> +{
> +}
> +
> +Process::~Process()
> +{
> + kill();
> + /* \todo wait for child process to exit */
> +}
> +
> +/**
> + * \brief Fork and exec a process, and close fds
> + * \param[in] path Path to executable
> + * \param[in] args Arguments to pass to executable (optional)
> + * \param[in] fds Vector of file descriptors to keep open (optional)
> + *
> + * Fork a process, and exec the executable specified by path. Prior to
> + * exec'ing, but after forking, all file descriptors except for those
> + * specified in fds will be closed.
> + *
> + * All indexes of args will be incremented by 1 before being fed to exec(),
> + * so args[0] should not need to be equal to path.
> + *
> + * \return zero on successful fork, exec, and closing the file descriptors,
> + * or a negative error code otherwise
> + */
> +int Process::start(const std::string &path,
> + const std::vector<std::string> &args,
> + const std::vector<int> &fds)
> +{
> + int ret;
> +
> + if (running_ == true)
You can drop == true.
> + return 0;
> +
> + int childPid = fork();
> + if (childPid == -1) {
> + ret = -errno;
> + LOG(Process, Error) << "Failed to fork: " << strerror(-ret);
> + return ret;
> + } else if (childPid) {
> + pid_ = childPid;
> + ProcessManager::instance()->registerProcess(this);
> +
> + running_ = true;
> +
> + return 0;
> + } else {
> + if (isolate())
> + _exit(EXIT_FAILURE);
> +
> + closeAllFdsExcept(fds);
> +
> + unsetenv("LIBCAMERA_LOG_FILE");
> +
> + const char **argv = new const char *[args.size() + 2];
> + unsigned int len = args.size();
> + argv[0] = path.c_str();
> + for (unsigned int i = 0; i < len; i++)
> + argv[i+1] = args[i].c_str();
> + argv[len+1] = nullptr;
> +
> + execv(path.c_str(), (char **)argv);
> +
> + exit(EXIT_FAILURE);
> + }
> +}
> +
> +void Process::closeAllFdsExcept(const std::vector<int> &fds)
> +{
> + std::vector<int> v(fds);
> + sort(v.begin(), v.end());
> +
> + 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()
> +{
> + return unshare(CLONE_NEWUSER | CLONE_NEWNET);
> +}
> +
> +/**
> + * \brief SIGCHLD handler
> + * \param[in] wstatus The status as output by waitpid()
> + *
> + * This method is called when the process associated with Process terminates.
> + * It emits the Process::finished signal.
> + */
> +void Process::died(int wstatus)
> +{
> + running_ = false;
> + exitStatus_ = WIFEXITED(wstatus) ? NormalExit : SignalExit;
> + exitCode_ = exitStatus_ == NormalExit ? WEXITSTATUS(wstatus) : -1;
> +
> + finished.emit(this, exitStatus_, exitCode_);
> +}
> +
> +/**
> + * \fn Process::exitStatus()
> + * \brief Retrieve the exit status of the process
> + *
> + * Return the exit status of the process, that is, whether the process
> + * has exited via exit() or returning from main, or if the process was
> + * terminated by a signal.
> + *
> + * \sa ExitStatus
> + *
> + * \return The process exit status
> + */
> +
> +/**
> + * \fn Process::exitCode()
> + * \brief Retrieve the exit code of the process
> + *
> + * This method is only valid if exitStatus() returned NormalExit.
> + *
> + * \return Exit code
> + */
> +
> +/**
> + * \var Process::finished
> + *
> + * Signal that is emitted when the process is confirmed to have terminated.
> + */
> +
> +/**
> + * \brief Kill the process
> + *
> + * Sends SIGKILL to the process.
> + */
> +void Process::kill()
> +{
> + ::kill(pid_, SIGKILL);
> +}
> +
> +} /* namespace libcamera */
> diff --git a/src/libcamera/process_manager.cpp b/src/libcamera/process_manager.cpp
> new file mode 100644
> index 0000000..e69de29
> diff --git a/test/meson.build b/test/meson.build
> index d308ac9..ad1a2f2 100644
> --- a/test/meson.build
> +++ b/test/meson.build
> @@ -6,6 +6,7 @@ subdir('ipa')
> subdir('ipc')
> subdir('media_device')
> subdir('pipeline')
> +subdir('process')
> subdir('stream')
> subdir('v4l2_subdevice')
> subdir('v4l2_videodevice')
> diff --git a/test/process/meson.build b/test/process/meson.build
> new file mode 100644
> index 0000000..c4d83d6
> --- /dev/null
> +++ b/test/process/meson.build
> @@ -0,0 +1,12 @@
> +process_tests = [
> + [ 'process_test', 'process_test.cpp' ],
> +]
> +
> +foreach t : process_tests
> + exe = executable(t[0], t[1],
> + dependencies : libcamera_dep,
> + link_with : test_libraries,
> + include_directories : test_includes_internal)
> +
> + test(t[0], exe, suite : 'process', is_parallel : false)
> +endforeach
> diff --git a/test/process/process_test.cpp b/test/process/process_test.cpp
> new file mode 100644
> index 0000000..8a7d09f
> --- /dev/null
> +++ b/test/process/process_test.cpp
> @@ -0,0 +1,100 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (C) 2019, Google Inc.
> + *
> + * process_test.cpp - Process test
> + */
> +
> +#include <iostream>
> +#include <unistd.h>
> +#include <vector>
> +
> +#include <libcamera/camera_manager.h>
> +#include <libcamera/event_dispatcher.h>
> +#include <libcamera/timer.h>
> +
> +#include "process.h"
> +#include "test.h"
> +#include "utils.h"
> +
> +using namespace std;
> +using namespace libcamera;
> +
> +class ProcessTestChild
> +{
> +public:
> + int run(int status)
> + {
> + usleep(50000);
> +
> + return status;
> + }
> +};
> +
> +class ProcessTest : public Test
> +{
> +public:
> + ProcessTest()
> + {
> + }
> +
> +protected:
> + int run()
> + {
> + EventDispatcher *dispatcher = CameraManager::instance()->eventDispatcher();
> + Timer timeout;
> +
> + int exitCode = 42;
> + vector<std::string> args;
> + args.push_back(to_string(exitCode));
> + int ret = proc_.start("/proc/self/exe", args);
> + if (ret) {
> + cerr << "failed to fork" << endl;
"failed to start process"
Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> + return TestFail;
> + }
> + proc_.finished.connect(this, &ProcessTest::procFinished);
> +
> + timeout.start(80);
> + while (timeout.isRunning())
> + dispatcher->processEvents();
> +
> + if (exitStatus_ != Process::NormalExit) {
> + cerr << "process did not exit normally" << endl;
> + return TestFail;
> + }
> +
> + if (exitCode != exitCode_) {
> + cerr << "exit code should be " << exitCode
> + << ", actual is " << exitCode_ << endl;
> + return TestFail;
> + }
> +
> + return TestPass;
> + }
> +
> +private:
> + void procFinished(Process *proc, enum Process::ExitStatus exitStatus, int exitCode)
> + {
> + exitStatus_ = exitStatus;
> + exitCode_ = exitCode;
> + }
> +
> + Process proc_;
> + enum Process::ExitStatus exitStatus_;
> + int exitCode_;
> +};
> +
> +/*
> + * Can't use TEST_REGISTER() as single binary needs to act as both
> + * parent and child processes.
> + */
> +int main(int argc, char **argv)
> +{
> + if (argc == 2) {
> + int status = std::stoi(argv[1]);
> + ProcessTestChild child;
> + return child.run(status);
> + }
> +
> + return ProcessTest().execute();
> +}
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list