[vte] widget: Fix race between polling the master & using the PTY in a child



commit 65d67f6f814df4f4ab800898bb7f4b1bc9f135b0
Author: Debarshi Ray <debarshir gnome org>
Date:   Fri Jun 22 13:19:56 2018 +0200

    widget: Fix race between polling the master & using the PTY in a child
    
    When a very short-lived process, like true(1), is spawned
    asynchronously as a child, there is a race between the child closing
    the pseudo-terminal's slave device on exit and VteTerminal receiving
    it, and the GAsyncReadyCallback passed to vte_pty_spawn_async being
    invoked. If the child closes it and the G_IO_HUP is received before the
    callback is invoked, then it causes VterTerminal to unset its VtePty
    object, which leads to the following CRITICAL when the callback tries
    to set up a watch on the child:
      Vte-CRITICAL **: void vte_terminal_watch_child(VteTerminal*, GPid):
        assertion 'impl->m_pty != NULL' failed
    
    The race can be avoided by setting up the GIOChannel to poll the
    pseudo-terminal master device only after the callback has been
    invoked. If the kernel has already buffered up some activity on the
    slave device, then that will be seen in the next iteration of the main
    loop, and the VtePty won't be unset before setting up the watch.
    
    There is a similar race for downstreams that continue to use
    gnome-pty-helper. In those cases, the helper is in charge of creating
    the pseudo-terminal device pair, and it closes its copies of the file
    descriptors after sending them to VteTerminal. If VteTerminal starts
    polling the master device immediately after receiving it from the
    helper, before the child process has been forked, and the helper loses
    the race to close its copy of the slave device's file descriptor
    before the master is polled, then VteTerminal will receive a G_IO_HUP
    and stop reading further input from the master. The subsequently forked
    child process gets left in a defunct state and the same CRITICAL is
    logged.
    
    This also makes vte_terminal_spawn_async match its synchronous variant,
    which is nice.
    
    Fixes GNOME/vte#7:
    https://gitlab.gnome.org/GNOME/vte/issues/7

 src/vtegtk.cc | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
---
diff --git a/src/vtegtk.cc b/src/vtegtk.cc
index 9e541be1..c435b55c 100644
--- a/src/vtegtk.cc
+++ b/src/vtegtk.cc
@@ -2543,10 +2543,12 @@ spawn_async_cb (GObject *source,
 
         /* Automatically watch the child */
         if (terminal != nullptr) {
-                if (pid != -1)
+                if (pid != -1) {
+                        vte_terminal_set_pty(terminal, pty);
                         vte_terminal_watch_child(terminal, pid);
-                else
+                } else {
                         vte_terminal_set_pty(terminal, nullptr);
+                }
         } else {
                 if (pid != -1) {
                         vte_reaper_add_child(pid);
@@ -2671,8 +2673,6 @@ vte_terminal_spawn_async(VteTerminal *terminal,
                 return;
         }
 
-        vte_terminal_set_pty(terminal, pty);
-
         guint spawn_flags = (guint)spawn_flags_;
 
         /* We do NOT support this flag. If you want to have some FD open in the child


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]