Work on centralising lifecycle management in the child monitor

kitty/boss.py

@@ -2,6 +2,7 @@
 # vim:fileencoding=utf-8
 # License: GPL v3 Copyright: 2016, Kovid Goyal <kovid at kovidgoyal.net>
 
+from functools import partial
 from gettext import gettext as _
 from time import monotonic
 from weakref import WeakValueDictionary
@@ -134,10 +135,20 @@ class Boss:
             yield from t
 
     def add_child(self, window):
-        self.child_monitor.add_child(window.id, window.child_fd, window.screen)
+        self.child_monitor.add_child(window.id, window.child.child_fd, window.screen)
         self.window_id_map[window.id] = window
         wakeup()
 
+    def retry_resize_pty(self, window_id, timers_call=False):
+        # In case the child has not yet been added in the child monitor
+        if timers_call:
+            w = self.window_id_map.get(window_id)
+            if w is not None:
+                if not self.child_monitor.resize_pty(window_id, *w.current_pty_size):
+                    self.retry_resize_pty(window_id)
+        else:
+            self.ui_timers.add(0, partial(self.retry_resize_pty, window_id, timers_call=True))
+
     def on_child_death(self, window_id):
         w = self.window_id_map.pop(window_id, None)
         if w is not None:
@@ -151,9 +162,7 @@ class Boss:
     def close_window(self, window=None):
         if window is None:
             window = self.active_window
-        self.child_monitor.mark_for_close(window.screen.child_fd)
+        self.child_monitor.mark_for_close(window.id)
-        self.gui_close_window()
-        window.destroy()
         wakeup()
 
     def close_tab(self, tab=None):

kitty/child-monitor.c

@@ -10,11 +10,15 @@
 #define _GNU_SOURCE
 #endif
 #include <pthread.h>
+#ifndef __APPLE__
 #undef _GNU_SOURCE
+#endif
 #include "data-types.h"
 #include <unistd.h>
 #include <float.h>
 #include <fcntl.h>
+#include <stropts.h>
+#include <sys/ioctl.h>
 #include <signal.h>
 #include <GLFW/glfw3.h>
 
@@ -41,8 +45,7 @@ static const Child EMPTY_CHILD = {0};
 static Child children[MAX_CHILDREN] = {{0}};
 static Child scratch[MAX_CHILDREN] = {{0}};
 static Child add_queue[MAX_CHILDREN] = {{0}}, remove_queue[MAX_CHILDREN] = {{0}};
-static unsigned long dead_children[MAX_CHILDREN] = {0};
+static unsigned long remove_notify[MAX_CHILDREN] = {0};
-static size_t num_dead_children = 0;
 static size_t add_queue_count = 0, remove_queue_count = 0;
 static struct pollfd fds[MAX_CHILDREN + EXTRA_FDS] = {{0}};
 static pthread_mutex_t children_lock = {{0}};
@@ -244,30 +247,31 @@ shutdown(ChildMonitor *self) {
 
 static inline void
 do_parse(ChildMonitor *self, Screen *screen, unsigned long child_id) {
+    bool updated = false;
     screen_mutex(lock, read);
     if (screen->read_buf_sz) {
         parse_func(screen, self->dump_callback);
         if (screen->read_buf_sz >= READ_BUF_SZ) wakeup_(wakeup_fds[1]);  // Ensure the read fd has POLLIN set
         screen->read_buf_sz = 0;
+        updated = true;
+    }
+    screen_mutex(unlock, read);
+    if (LIKELY(updated)) {
         PyObject *t = PyObject_CallFunction(self->update_screen, "k", child_id);
         if (t == NULL) PyErr_Print();
         else Py_DECREF(t);
     }
-    screen_mutex(unlock, read);
 }
 
 static void
 parse_input(ChildMonitor *self) {
     // Parse all available input that was read in the I/O thread.
-    size_t count = 0;
+    size_t count = 0, remove_count = 0;
     children_mutex(lock);
-    while (num_dead_children) {
-        PyObject *t = PyObject_CallFunction(self->death_notify, "k", dead_children[--num_dead_children]);
-        if (t == NULL) PyErr_Print();
-        else Py_DECREF(t);
-    }
     while (remove_queue_count) {
-        remove_queue_count--;
+        remove_queue_count--; 
+        remove_notify[remove_count] = remove_queue[remove_queue_count].id;
+        remove_count++;
         FREE_CHILD(remove_queue[remove_queue_count]);
     }
 
@@ -283,6 +287,15 @@ parse_input(ChildMonitor *self) {
     }
     children_mutex(unlock);
 
+    while(remove_count) {
+        // must be done while no locks are held, since the locks are non-recursive and
+        // the python function could call into other functions in this module
+        remove_count--;
+        PyObject *t = PyObject_CallFunction(self->death_notify, "k", remove_notify[remove_count]);
+        if (t == NULL) PyErr_Print();
+        else Py_DECREF(t);
+    }
+
     double wait_for = self->repaint_delay;
     for (size_t i = 0; i < count; i++) {
         if (!scratch[i].needs_removal) {
@@ -305,11 +318,11 @@ parse_input(ChildMonitor *self) {
 static PyObject *
 mark_for_close(ChildMonitor *self, PyObject *args) {
 #define mark_for_close_doc "Mark a child to be removed from the child monitor"
-    int fd;
+    unsigned long window_id;
-    if (!PyArg_ParseTuple(args, "i", &fd)) return NULL;
+    if (!PyArg_ParseTuple(args, "k", &window_id)) return NULL;
     children_mutex(lock);
     for (size_t i = 0; i < self->count; i++) {
-        if (children[i].fd == fd) {
+        if (children[i].id == window_id) {
             children[i].needs_removal = true;
             break;
         }
@@ -318,6 +331,27 @@ mark_for_close(ChildMonitor *self, PyObject *args) {
     Py_RETURN_NONE;
 }
 
+static PyObject *
+resize_pty(ChildMonitor *self, PyObject *args) {
+#define resize_pty_doc "Resize the pty associated with the specified child"
+    unsigned long window_id;
+    struct winsize dim;
+    PyObject *found = Py_False;
+    if (!PyArg_ParseTuple(args, "kHHHH", &window_id, &dim.ws_row, &dim.ws_col, &dim.ws_xpixel, &dim.ws_ypixel)) return NULL;
+    children_mutex(lock);
+    for (size_t i = 0; i < self->count; i++) {
+        if (children[i].id == window_id) {
+            found = Py_True;
+            if (ioctl(fds[EXTRA_FDS + i].fd, TIOCSWINSZ, &dim) == -1) PyErr_SetFromErrno(PyExc_OSError);
+            break;
+        }
+    }
+    children_mutex(unlock);
+    if (PyErr_Occurred()) return NULL;
+    Py_INCREF(found);
+    return found;
+}
+
 #undef FREE_CHILD
 #undef INCREF_CHILD
 #undef DECREF_CHILD
@@ -507,9 +541,9 @@ io_loop(void *data) {
                     data_received = true;
                     has_more = read_bytes(fds[EXTRA_FDS + i].fd, children[i].screen);
                     if (!has_more) { 
+                        // child is dead
                         children_mutex(lock);
                         children[i].needs_removal = true;
-                        dead_children[num_dead_children++] = children[i].id;
                         children_mutex(unlock);
                     }
                 }
@@ -527,7 +561,6 @@ io_loop(void *data) {
     children_mutex(lock);
     for (i = 0; i < self->count; i++) children[i].needs_removal = true;
     remove_children(self);
-    for (i = 0; i < EXTRA_FDS; i++) close(fds[i].fd);
     children_mutex(unlock);
     return 0;
 }
@@ -543,6 +576,7 @@ static PyMethodDef methods[] = {
     METHOD(shutdown, METH_NOARGS)
     METHOD(main_loop, METH_NOARGS)
     METHOD(mark_for_close, METH_VARARGS)
+    METHOD(resize_pty, METH_VARARGS)
     {NULL}  /* Sentinel */
 };
 

kitty/child.py

@@ -4,7 +4,6 @@
 
 import os
 import termios
-import struct
 import fcntl
 import signal
 from threading import Thread
@@ -77,10 +76,6 @@ class Child:
                 t.start()
             return pid
 
-    def resize_pty(self, w, h, ww, wh):
-        if self.child_fd is not None:
-            fcntl.ioctl(self.child_fd, termios.TIOCSWINSZ, struct.pack('4H', h, w, ww, wh))
-
     def set_iutf8(self, on=True, fd=None):
         fd = fd or self.child_fd
         if fd is not None and hasattr(fast_data_types, 'IUTF8'):

kitty/tabs.py

@@ -193,10 +193,7 @@ class Tab:
         return window in self.windows
 
     def destroy(self):
-        if hasattr(self, 'windows'):
+        self.windows = deque()
-            for w in self.windows:
-                w.destroy()
-            del self.windows
 
     def render(self):
         self.borders.render(get_boss().borders_program)

kitty/window.py

@@ -50,13 +50,13 @@ class Window:
         self.title = appname
         self._is_visible_in_layout = True
         self.child, self.opts = child, opts
-        self.child_fd = child.child_fd
         self.start_visual_bell_at = None
         self.screen = Screen(self, 24, 80, opts.scrollback_lines)
         self.char_grid = CharGrid(self.screen, opts)
+        self.current_pty_size = None
 
     def __repr__(self):
-        return 'Window(title={}, id={})'.format(self.title, hex(id(self)))
+        return 'Window(title={}, id={})'.format(self.title, self.id)
 
     @property
     def is_visible_in_layout(self):
@@ -75,12 +75,19 @@ class Window:
         wakeup()
 
     def set_geometry(self, new_geometry):
+        if self.destroyed:
+            return
         if self.needs_layout or new_geometry.xnum != self.screen.columns or new_geometry.ynum != self.screen.lines:
+            boss = get_boss()
+            child_monitor = boss.child_monitor
             self.screen.resize(new_geometry.ynum, new_geometry.xnum)
-            self.child.resize_pty(self.screen.columns, self.screen.lines,
+            self.current_pty_size = (
-                                  max(0, new_geometry.right - new_geometry.left), max(0, new_geometry.bottom - new_geometry.top))
+                self.screen.lines, self.screen.columns,
+                max(0, new_geometry.right - new_geometry.left), max(0, new_geometry.bottom - new_geometry.top))
             self.char_grid.resize(new_geometry)
             self.needs_layout = False
+            if not child_monitor.resize_pty(self.id, *self.current_pty_size):
+                boss.retry_resize_pty(self.id)
         else:
             self.char_grid.update_position(new_geometry)
         self.geometry = new_geometry
@@ -93,19 +100,17 @@ class Window:
         get_boss().close_window(self)
 
     def on_child_death(self):
-        self.destroy()
+        if self.destroyed:
-        get_boss().gui_close_window(self)
+            return
+        self.destroyed = True
+        self.child.hangup()
+        self.child.get_child_status()  # Ensure child does not become zombie
         # Remove cycles so that screen is de-allocated immediately
-        if self.screen is not None:
+        boss = get_boss()
-            self.screen.reset_callbacks()
+        self.screen.reset_callbacks()
-            self.screen = self.char_grid.screen = None
+        boss.gui_close_window(self)
-            self.char_grid = None
+        self.screen = self.char_grid.screen = None
-
+        self.char_grid = None
-    def destroy(self):
-        if not self.destroyed:
-            self.destroyed = True
-            self.child.hangup()
-            self.child.get_child_status()  # Ensure child does not become zombie
 
     def write_to_child(self, data):
         if data: