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

@@ -0,0 +1,87 @@
+/*
+ * child.c
+ * Copyright (C) 2018 Kovid Goyal <kovid at kovidgoyal.net>
+ *
+ * Distributed under terms of the GPL3 license.
+ */
+
+#include "data-types.h"
+#include <unistd.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+#include <errno.h>
+#include <string.h>
+
+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;
+}

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
+        else:
-        pid = os.fork()
+            stdin_read_fd = stdin_write_fd = -1
-        if pid == 0:  # child
+        env = os.environ.copy()
-            try:
+        env.update(self.env)
-                os.chdir(self.cwd)
+        env['TERM'] = self.opts.term
-            except EnvironmentError:
+        env['COLORTERM'] = 'truecolor'
-                os.chdir('/')
+        if os.path.isdir(terminfo_dir):
-            os.setsid()
+            env['TERMINFO'] = terminfo_dir
-            for i in range(3):
+        env = tuple('{}={}'.format(k, v) for k, v in env.items())
-                if stdin is not None and i == 0:
+        pid = fast_data_types.spawn(self.cwd, tuple(self.argv), env, master, slave, stdin_read_fd, stdin_write_fd)
-                    os.dup2(stdin_read_fd, i)
+        os.close(slave)
-                    os.close(stdin_read_fd), os.close(stdin_write_fd)
+        self.pid = pid
-                else:
+        self.child_fd = master
-                    os.dup2(slave, i)
+        if stdin is not None:
-            os.close(slave), os.close(master)
+            os.close(stdin_read_fd)
-            os.closerange(3, 200)
+            fast_data_types.thread_write(stdin_write_fd, stdin)
-            # Establish the controlling terminal (see man 7 credentials)
+        return pid
-            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

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;