From 239eb8202bcfae1ad22d60241a8f734b67ca975b Mon Sep 17 00:00:00 2001 From: Kovid Goyal Date: Thu, 4 Jan 2018 23:19:09 +0530 Subject: [PATCH] Move fork()+exec() of child processes into C By avoiding python in the child process before exec we ensure that malloc and other unsafe to use after fork functions are not used. Should also mean that less pages will need to be copied into thec hild process, leading to marginally faster startups. --- kitty/child.c | 87 ++++++++++++++++++++++++++++++++++++++++++++++ kitty/child.py | 59 +++++++++---------------------- kitty/data-types.c | 2 ++ 3 files changed, 106 insertions(+), 42 deletions(-) create mode 100644 kitty/child.c diff --git a/kitty/child.c b/kitty/child.c new file mode 100644 index 000000000..48ea67683 --- /dev/null +++ b/kitty/child.c @@ -0,0 +1,87 @@ +/* + * child.c + * Copyright (C) 2018 Kovid Goyal + * + * Distributed under terms of the GPL3 license. + */ + +#include "data-types.h" +#include +#include +#include +#include +#include + +static inline char** +serialize_string_tuple(PyObject *src) { + Py_ssize_t sz = PyTuple_GET_SIZE(src); + char **ans = calloc(sz + 1, sizeof(char*)); + if (!ans) fatal("Out of memory"); + for (Py_ssize_t i = 0; i < sz; i++) ans[i] = PyUnicode_AsUTF8(PyTuple_GET_ITEM(src, i)); + return ans; +} + +extern char **environ; + +static PyObject* +spawn(PyObject *self UNUSED, PyObject *args) { + PyObject *argv_p, *env_p; + int master, slave, stdin_read_fd, stdin_write_fd; + char* cwd; + if (!PyArg_ParseTuple(args, "sO!O!iiii", &cwd, &PyTuple_Type, &argv_p, &PyTuple_Type, &env_p, &master, &slave, &stdin_read_fd, &stdin_write_fd)) return NULL; + char **argv = serialize_string_tuple(argv_p); + char **env = serialize_string_tuple(env_p); + + pid_t pid = fork(); + if (pid == 0) { + // child + // We cannot use malloc before exec() as it might deadlock if a thread in the parent process is in the middle of a malloc itself + if (chdir(cwd) != 0) chdir("/"); + if (setsid() == -1) { perror("setsid() in child process failed"); exit(EXIT_FAILURE); } + if (dup2(slave, 1) == -1) { perror("dup2() failed for fd number 1"); exit(EXIT_FAILURE); } + if (dup2(slave, 2) == -1) { perror("dup2() failed for fd number 2"); exit(EXIT_FAILURE); } + if (stdin_read_fd > -1) { + if (dup2(stdin_read_fd, 0) == -1) { perror("dup2() failed for fd number 0"); exit(EXIT_FAILURE); } + close(stdin_read_fd); + close(stdin_write_fd); + } else { + if (dup2(slave, 0) == -1) { perror("dup2() failed for fd number 0"); exit(EXIT_FAILURE); } + } + close(slave); + close(master); + for (int c = 3; c < 201; c++) close(c); + + // Establish the controlling terminal (see man 7 credentials) + char *name = ttyname(1); + if (name == NULL) { perror("Failed to call ttyname()"); exit(EXIT_FAILURE); } + int tfd = open(name, O_RDWR); + if (tfd == -1) { perror("Failed to open controlling terminal"); exit(EXIT_FAILURE); } + close(tfd); + + environ = env; + execvp(argv[0], argv); + // Report the failure and exec a shell instead, so that we are not left + // with a forked but not execed process + fprintf(stderr, "Failed to launch child: %s\nWith error: %s [%d]\n", argv[0], strerror(errno), errno); + fprintf(stderr, "Press Enter to exit.\n"); + fflush(stderr); + execlp("sh", "sh", "-c", "read w", NULL); + exit(EXIT_FAILURE); + } else { + free(argv); + free(env); + } + return PyLong_FromLong(pid); +} + +static PyMethodDef module_methods[] = { + METHODB(spawn, METH_VARARGS), + {NULL, NULL, 0, NULL} /* Sentinel */ +}; + + +bool +init_child(PyObject *module) { + if (PyModule_AddFunctions(module, module_methods) != 0) return false; + return true; +} diff --git a/kitty/child.py b/kitty/child.py index 8d9df44a4..c7acaf639 100644 --- a/kitty/child.py +++ b/kitty/child.py @@ -4,7 +4,6 @@ import fcntl import os -import sys import kitty.fast_data_types as fast_data_types @@ -55,44 +54,20 @@ class Child: if stdin is not None: stdin_read_fd, stdin_write_fd = os.pipe() remove_cloexec(stdin_read_fd) - env = self.env - pid = os.fork() - if pid == 0: # child - try: - os.chdir(self.cwd) - except EnvironmentError: - os.chdir('/') - os.setsid() - for i in range(3): - if stdin is not None and i == 0: - os.dup2(stdin_read_fd, i) - os.close(stdin_read_fd), os.close(stdin_write_fd) - else: - os.dup2(slave, i) - os.close(slave), os.close(master) - os.closerange(3, 200) - # Establish the controlling terminal (see man 7 credentials) - os.close(os.open(os.ttyname(1), os.O_RDWR)) - os.environ.update(env) - os.environ['TERM'] = self.opts.term - os.environ['COLORTERM'] = 'truecolor' - if os.path.isdir(terminfo_dir): - os.environ['TERMINFO'] = terminfo_dir - try: - os.execvp(self.argv[0], self.argv) - except Exception as err: - # Report he failure and exec a shell instead so that - # we are not left with a forked but not execed process - print('Could not launch:', self.argv[0]) - print('\t', err) - print('\nPress Enter to exit:', end=' ') - sys.stdout.flush() - os.execvp('/bin/sh', ['/bin/sh', '-c', 'read w']) - else: # master - os.close(slave) - self.pid = pid - self.child_fd = master - if stdin is not None: - os.close(stdin_read_fd) - fast_data_types.thread_write(stdin_write_fd, stdin) - return pid + else: + stdin_read_fd = stdin_write_fd = -1 + env = os.environ.copy() + env.update(self.env) + env['TERM'] = self.opts.term + env['COLORTERM'] = 'truecolor' + if os.path.isdir(terminfo_dir): + env['TERMINFO'] = terminfo_dir + env = tuple('{}={}'.format(k, v) for k, v in env.items()) + pid = fast_data_types.spawn(self.cwd, tuple(self.argv), env, master, slave, stdin_read_fd, stdin_write_fd) + os.close(slave) + self.pid = pid + self.child_fd = master + if stdin is not None: + os.close(stdin_read_fd) + fast_data_types.thread_write(stdin_write_fd, stdin) + return pid diff --git a/kitty/data-types.c b/kitty/data-types.c index 96938346e..d4199cdc3 100644 --- a/kitty/data-types.c +++ b/kitty/data-types.c @@ -180,6 +180,7 @@ extern bool init_fontconfig_library(PyObject*); extern bool init_desktop(PyObject*); extern bool init_fonts(PyObject*); extern bool init_glfw(PyObject *m); +extern bool init_child(PyObject *m); extern bool init_state(PyObject *module); extern bool init_keys(PyObject *module); extern bool init_graphics(PyObject *module); @@ -211,6 +212,7 @@ PyInit_fast_data_types(void) { if (!init_ColorProfile(m)) return NULL; if (!init_Screen(m)) return NULL; if (!init_glfw(m)) return NULL; + if (!init_child(m)) return NULL; if (!init_state(m)) return NULL; if (!init_keys(m)) return NULL; if (!init_graphics(m)) return NULL;