Do not use a global variable to store child data

kitty/boss.py

@@ -19,7 +19,7 @@ import glfw
 from .constants import appname
 from .char_grid import CharGrid
 from .keys import interpret_text_event, interpret_key_event
-from .utils import resize_pty, create_pty, sanitize_title
+from .utils import sanitize_title
 from .fast_data_types import (
     BRACKETED_PASTE_START, BRACKETED_PASTE_END, Screen, read_bytes_dump, read_bytes
 )
@@ -41,12 +41,14 @@ class Boss(Thread):
     pending_title_change = pending_icon_change = None
     pending_color_changes = {}
 
-    def __init__(self, window, window_width, window_height, opts, args):
+    def __init__(self, window, window_width, window_height, opts, args, child):
         Thread.__init__(self, name='ChildMonitor')
+        self.child = child
         self.screen_update_delay = opts.repaint_delay / 1000.0
         self.pending_update_screen = None
         self.action_queue = Queue()
-        self.child_fd = create_pty()[0]
+        self.child.fork()
+        self.child_fd = self.child.child_fd
         self.read_wakeup_fd, self.write_wakeup_fd = os.pipe2(os.O_NONBLOCK | os.O_CLOEXEC)
         self.signal_fd = handle_unix_signals()
         self.readers = [self.child_fd, self.signal_fd, self.read_wakeup_fd]
@@ -132,7 +134,7 @@ class Boss(Thread):
     def apply_resize_screen(self, w, h):
         self.char_grid.resize_screen(w, h)
         sg = self.char_grid.screen_geometry
-        resize_pty(sg.xnum, sg.ynum)
+        self.child.resize_pty(sg.xnum, sg.ynum)
         glfw.glfwPostEmptyEvent()
 
     def apply_opts(self, opts):
@@ -194,6 +196,8 @@ class Boss(Thread):
         signal.signal(signal.SIGINT, signal.SIG_DFL)
         signal.signal(signal.SIGTERM, signal.SIG_DFL)
         self.char_grid.destroy()
+        self.child.hangup()
+        self.child.get_child_status()  # Ensure child does not become zombie
 
     def shutdown(self):
         self.shutting_down = True

kitty/child.py

@@ -0,0 +1,83 @@
+#!/usr/bin/env python
+# vim:fileencoding=utf-8
+# License: GPL v3 Copyright: 2016, Kovid Goyal <kovid at kovidgoyal.net>
+
+import os
+import termios
+import struct
+import fcntl
+import signal
+
+from .constants import terminfo_dir
+
+
+class Child:
+
+    child_fd = pid = None
+    forked = False
+
+    def __init__(self, argv, cwd, opts):
+        self.argv = argv
+        self.cwd = cwd
+        self.opts = opts
+
+    def fork(self):
+        if self.forked:
+            return
+        self.forked = True
+        master, slave = os.openpty()
+        fcntl.fcntl(slave, fcntl.F_SETFD, fcntl.fcntl(slave, fcntl.F_GETFD) & ~fcntl.FD_CLOEXEC)
+        # Note that master and slave are in blocking mode
+        pid = os.fork()
+        if pid == 0:  # child
+            try:
+                os.chdir(self.cwd)
+            except EnvironmentError:
+                os.chdir('/')
+            os.setsid()
+            for i in range(3):
+                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['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:
+                print('Could not launch:', self.argv[0])
+                print('\t', err)
+                input('\nPress Enter to exit:')
+        else:  # master
+            os.close(slave)
+            self.pid = pid
+            self.child_fd = master
+            return pid
+
+    def resize_pty(self, w, h):
+        if self.child_fd is not None:
+            fcntl.ioctl(self.child_fd, termios.TIOCSWINSZ, struct.pack('4H', h, w, 0, 0))
+
+    def hangup(self):
+        if self.pid is not None:
+            pid, self.pid = self.pid, None
+            try:
+                pgrp = os.getpgid(pid)
+            except ProcessLookupError:
+                return
+            os.killpg(pgrp, signal.SIGHUP)
+            os.close(self.child_fd)
+            self.child_fd = None
+
+    def __del__(self):
+        self.hangup()
+
+    def get_child_status(self):
+        if self.pid is not None:
+            try:
+                return os.waitid(os.P_PID, self.pid, os.WEXITED | os.WNOHANG)
+            except ChildProcessError:
+                self.pid = None

kitty/main.py

@@ -10,10 +10,10 @@ import pwd
 from gettext import gettext as _
 
 
+from .child import Child
 from .config import load_config
 from .constants import appname, str_version, config_dir
 from .boss import Boss
-from .utils import fork_child, hangup, get_child_status
 from .shaders import GL_VERSION
 from .fast_data_types import glewInit, enable_automatic_opengl_error_checking
 import glfw
@@ -44,7 +44,7 @@ def setup_opengl():
     glfw.glfwWindowHint(glfw.GLFW_SAMPLES, 0)
 
 
-def run_app(opts, args):
+def run_app(opts, args, child):
     setup_opengl()
     window_width = window_height = 1024
     window = glfw.glfwCreateWindow(
@@ -56,7 +56,7 @@ def run_app(opts, args):
         glfw.glfwMakeContextCurrent(window)
         glewInit()
         glfw.glfwSwapInterval(1)
-        boss = Boss(window, window_width, window_height, opts, args)
+        boss = Boss(window, window_width, window_height, opts, args, child)
         glfw.glfwSetFramebufferSizeCallback(window, boss.on_window_resize)
         boss.start()
         try:
@@ -69,7 +69,6 @@ def run_app(opts, args):
                 boss.close()
                 boss.join()
             boss.destroy()
-            get_child_status()  # Ensure child does not become zombie
     finally:
         glfw.glfwDestroyWindow(window)
 
@@ -90,7 +89,7 @@ def main():
         return
     opts = load_config(args.config)
     child = args.args or [pwd.getpwuid(os.geteuid()).pw_shell or '/bin/sh']
-    fork_child(child, args.directory, opts)
+    child = Child(child, args.directory, opts)
     glfw.glfwSetErrorCallback(on_glfw_error)
     enable_automatic_opengl_error_checking(False)
     if not glfw.glfwInit():
@@ -103,7 +102,7 @@ def main():
             import pstats
             pr = cProfile.Profile()
             pr.enable()
-            run_app(opts, args)
+            run_app(opts, args, child)
             pr.disable()
             pr.create_stats()
             s = pstats.Stats(pr)
@@ -113,8 +112,7 @@ def main():
             s.sort_stats('time', 'name')
             s.print_stats(30)
         else:
-            run_app(opts, args)
+            run_app(opts, args, child)
     finally:
         glfw.glfwTerminate()
-        hangup()
         os.closerange(3, 100)

kitty/utils.py

@@ -2,20 +2,13 @@
 # vim:fileencoding=utf-8
 # License: GPL v3 Copyright: 2016, Kovid Goyal <kovid at kovidgoyal.net>
 
-import os
 import re
 import subprocess
-import sys
-import termios
-import struct
-import fcntl
-import signal
 import ctypes
 from contextlib import contextmanager
 from functools import lru_cache
 from time import monotonic
 
-from .constants import terminfo_dir
 
 libc = ctypes.CDLL(None)
 wcwidth_native = libc.wcwidth
@@ -32,78 +25,6 @@ def wcwidth(c: str) -> int:
     return ans
 
 
-def create_pty():
-    if not hasattr(create_pty, 'master'):
-        create_pty.master, create_pty.slave = os.openpty()
-        fcntl.fcntl(create_pty.slave, fcntl.F_SETFD, fcntl.fcntl(create_pty.slave, fcntl.F_GETFD) & ~fcntl.FD_CLOEXEC)
-        # Note that master and slave are in blocking mode
-    return create_pty.master, create_pty.slave
-
-
-def fork_child(argv, cwd, opts):
-    master, slave = create_pty()
-    pid = os.fork()
-    if pid == 0:
-        try:
-            os.chdir(cwd)
-        except EnvironmentError:
-            os.chdir('/')
-        os.setsid()
-        for i in range(3):
-            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['TERM'] = opts.term
-        os.environ['COLORTERM'] = 'truecolor'
-        if os.path.isdir(terminfo_dir):
-            os.environ['TERMINFO'] = terminfo_dir
-        try:
-            os.execvp(argv[0], argv)
-        except Exception as err:
-            print('Could not launch:', argv[0])
-            print('\t', err)
-            input('\nPress Enter to exit:')
-    else:
-        os.close(slave)
-        fork_child.pid = pid
-        return pid
-
-
-def resize_pty(w, h):
-    master = create_pty()[0]
-    fcntl.ioctl(master, termios.TIOCSWINSZ, struct.pack('4H', h, w, 0, 0))
-
-
-def hangup():
-    if hasattr(fork_child, 'pid'):
-        pid = fork_child.pid
-        del fork_child.pid
-        try:
-            pgrp = os.getpgid(pid)
-        except ProcessLookupError:
-            return
-        os.killpg(pgrp, signal.SIGHUP)
-        os.close(create_pty()[0])
-
-
-def get_child_status():
-    if hasattr(fork_child, 'pid'):
-        try:
-            return os.waitid(os.P_PID, fork_child.pid, os.WEXITED | os.WNOHANG)
-        except ChildProcessError:
-            del fork_child.pid
-
-
-base_size = sys.getsizeof('')
-
-
-def is_simple_string(x):
-    ' We use the fact that python stores unicode strings with a 1-byte representation when possible '
-    return sys.getsizeof(x) == base_size + len(x)
-
-
 @contextmanager
 def timeit(name, do_timing=False):
     if do_timing: