[libcamera-devel,v4,3/8] libcamera: Add Process and ProcessManager classes

1662 diff mbox series

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@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

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@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@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();
> +}