From 5972a37550836449c5de2eff78f67eaae374b103 Mon Sep 17 00:00:00 2001 From: Kovid Goyal Date: Thu, 7 Sep 2017 13:44:12 +0530 Subject: [PATCH] Work on centralising lifecycle management in the child monitor --- kitty/boss.py | 17 +++++++++--- kitty/child-monitor.c | 64 +++++++++++++++++++++++++++++++++---------- kitty/child.py | 5 ---- kitty/tabs.py | 5 +--- kitty/window.py | 37 ++++++++++++++----------- 5 files changed, 84 insertions(+), 44 deletions(-) diff --git a/kitty/boss.py b/kitty/boss.py index bd8c94560..f49a25b5a 100644 --- a/kitty/boss.py +++ b/kitty/boss.py @@ -2,6 +2,7 @@ # vim:fileencoding=utf-8 # License: GPL v3 Copyright: 2016, Kovid Goyal +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.gui_close_window() - window.destroy() + self.child_monitor.mark_for_close(window.id) wakeup() def close_tab(self, tab=None): diff --git a/kitty/child-monitor.c b/kitty/child-monitor.c index fdb837d69..5f97a1087 100644 --- a/kitty/child-monitor.c +++ b/kitty/child-monitor.c @@ -10,11 +10,15 @@ #define _GNU_SOURCE #endif #include +#ifndef __APPLE__ #undef _GNU_SOURCE +#endif #include "data-types.h" #include #include #include +#include +#include #include #include @@ -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 size_t num_dead_children = 0; +static unsigned long remove_notify[MAX_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; - if (!PyArg_ParseTuple(args, "i", &fd)) return NULL; + unsigned long window_id; + 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 */ }; diff --git a/kitty/child.py b/kitty/child.py index a7dcb3d68..bf1c16d83 100644 --- a/kitty/child.py +++ b/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'): diff --git a/kitty/tabs.py b/kitty/tabs.py index 55fba8437..8108306ed 100644 --- a/kitty/tabs.py +++ b/kitty/tabs.py @@ -193,10 +193,7 @@ class Tab: return window in self.windows def destroy(self): - if hasattr(self, 'windows'): - for w in self.windows: - w.destroy() - del self.windows + self.windows = deque() def render(self): self.borders.render(get_boss().borders_program) diff --git a/kitty/window.py b/kitty/window.py index 94663d22c..664db4eaf 100644 --- a/kitty/window.py +++ b/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, - max(0, new_geometry.right - new_geometry.left), max(0, new_geometry.bottom - new_geometry.top)) + self.current_pty_size = ( + 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() - get_boss().gui_close_window(self) + if self.destroyed: + 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: - self.screen.reset_callbacks() - 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 + boss = get_boss() + self.screen.reset_callbacks() + boss.gui_close_window(self) + self.screen = self.char_grid.screen = None + self.char_grid = None def write_to_child(self, data): if data: