update controlling terminal and process group

xonsh/built_ins.py

@@ -27,7 +27,7 @@ from xonsh.inspectors import Inspector
 from xonsh.aliases import Aliases, make_default_aliases
 from xonsh.environ import Env, default_env, locate_binary
 from xonsh.foreign_shells import load_foreign_aliases
-from xonsh.jobs import add_job
+from xonsh.jobs import add_job, give_terminal_to
 from xonsh.platform import ON_POSIX, ON_WINDOWS
 from xonsh.proc import (
     PopenThread, ProcProxyThread, ProcProxy, ConsoleParallelReader,
@@ -496,6 +496,8 @@ class SubprocSpec:
         self.prep_env(kwargs)
         self.prep_preexec_fn(kwargs, pipeline_group=pipeline_group)
         if callable(self.alias):
+            if 'preexec_fn' in kwargs:
+                kwargs.pop('preexec_fn')
             p = self.cls(self.alias, self.cmd, **kwargs)
         else:
             p = self._run_binary(kwargs)
@@ -533,7 +535,7 @@ class SubprocSpec:
 
     def prep_preexec_fn(self, kwargs, pipeline_group=None):
         """Prepares the 'preexec_fn' keyword argument"""
-        if not (ON_POSIX and self.cls is subprocess.Popen):
+        if not ON_POSIX:
             return
         if not builtins.__xonsh_env__.get('XONSH_INTERACTIVE'):
             return
@@ -778,6 +780,17 @@ def _should_set_title(captured=False):
             hasattr(builtins, '__xonsh_shell__'))
 
 
+def update_fg_process_group(pipeline_group, background):
+    if background:
+        return False
+    if not ON_POSIX:
+        return False
+    env = builtins.__xonsh_env__
+    if not env.get('XONSH_INTERACTIVE'):
+        return False
+    return give_terminal_to(pipeline_group)
+
+
 def run_subproc(cmds, captured=False):
     """Runs a subprocess, in its many forms. This takes a list of 'commands,'
     which may be a list of command line arguments or a string, representing
@@ -793,34 +806,37 @@ def run_subproc(cmds, captured=False):
     """
     specs = cmds_to_specs(cmds, captured=captured)
     captured = specs[-1].captured
+    background = specs[-1].background
     procs = []
     proc = pipeline_group = None
+    term_pgid = None
     for spec in specs:
         starttime = time.time()
         proc = spec.run(pipeline_group=pipeline_group)
-        procs.append(proc)
+        if captured != 'object' and proc.pid and pipeline_group is None:
-        if ON_POSIX and pipeline_group is None and \
-           spec.cls is subprocess.Popen:
             pipeline_group = proc.pid
-    if not spec.is_proxy:
+            if update_fg_process_group(pipeline_group, background):
+                term_pgid = pipeline_group
+        procs.append(proc)
+    if not all(x.is_proxy for x in specs):
         add_job({
             'cmds': cmds,
             'pids': [i.pid for i in procs],
             'obj': proc,
-            'bg': spec.background,
+            'bg': background,
         })
     if _should_set_title(captured=captured):
         # set title here to get currently executing command
         pause_call_resume(proc, builtins.__xonsh_shell__.settitle)
     # create command or return if backgrounding.
-    if spec.background:
+    if background:
         return
     if captured == 'hiddenobject':
         command = HiddenCommandPipeline(specs, procs, starttime=starttime,
-                                        captured=captured)
+                                        captured=captured, term_pgid=term_pgid)
     else:
         command = CommandPipeline(specs, procs, starttime=starttime,
-                                  captured=captured)
+                                  captured=captured, term_pgid=term_pgid)
     # now figure out what we should return.
     if captured == 'stdout':
         command.end()

xonsh/jobs.py

@@ -28,7 +28,10 @@ if ON_DARWIN:
         for pid in job['pids']:
             if pid is None:  # the pid of an aliased proc is None
                 continue
-            os.kill(pid, signal)
+            try:
+                os.kill(pid, signal)
+            except ProcessLookupError:
+                pass
 elif ON_WINDOWS:
     pass
 elif ON_CYGWIN:
@@ -70,6 +73,9 @@ if ON_WINDOWS:
     def _set_pgrp(info):
         pass
 
+    def give_terminal_to(pgid):
+        pass
+
     def wait_for_active_job(last_task=None, backgrounded=False):
         """
         Wait for the active job to finish, to be killed by SIGINT, or to be
@@ -102,15 +108,15 @@ else:
         signal.signal(signal.SIGTSTP, signal.SIG_IGN)
 
     def _set_pgrp(info):
-        pid = info['pids'][0]
+        for pid in info['pids']:
-        if pid is None:
+            if pid is None:  # occurs if first process is an alias
-            # occurs if first process is an alias
+                continue
-            info['pgrp'] = None
+            try:
-            return
+                info['pgrp'] = os.getpgid(pid)
-        try:
+                return
-            info['pgrp'] = os.getpgid(pid)
+            except ProcessLookupError:
-        except ProcessLookupError:
+                continue
-            info['pgrp'] = None
+        info['pgrp'] = None
 
     _shell_pgrp = os.getpgrp()
 
@@ -123,21 +129,21 @@ else:
     def _shell_tty():
         try:
             _st = sys.stderr.fileno()
-            if os.tcgetpgrp(_st) != os.getpgid(os.getpid()):
+            if os.tcgetpgrp(_st) != os.getpgid(0):
                 # we don't own it
                 _st = None
         except OSError:
             _st = None
         return _st
 
-    # _give_terminal_to is a simplified version of:
+    # give_terminal_to is a simplified version of:
     #    give_terminal_to from bash 4.3 source, jobs.c, line 4030
     # this will give the terminal to the process group pgid
     if ON_CYGWIN:
         # on cygwin, signal.pthread_sigmask does not exist in Python, even
         # though pthread_sigmask is defined in the kernel.  thus, we use
         # ctypes to mimic the calls in the "normal" version below.
-        def _give_terminal_to(pgid):
+        def give_terminal_to(pgid):
             st = _shell_tty()
             if st is not None and os.isatty(st):
                 omask = ctypes.c_ulong()
@@ -153,12 +159,16 @@ else:
                 LIBC.sigprocmask(ctypes.c_int(signal.SIG_SETMASK),
                                  ctypes.byref(omask), None)
     else:
-        def _give_terminal_to(pgid):
+        def give_terminal_to(pgid):
-            st = _shell_tty()
+            oldmask = signal.pthread_sigmask(signal.SIG_BLOCK, _block_when_giving)
-            if st is not None and os.isatty(st):
+            try:
-                oldmask = signal.pthread_sigmask(signal.SIG_BLOCK,
+                os.tcsetpgrp(sys.stderr.fileno(), pgid)
-                                                 _block_when_giving)
+                return True
-                os.tcsetpgrp(st, pgid)
+            except Exception as e:
+                print('tcsetpgrp error {}: {}'.format(e.__class__.__name__, e),
+                      file=sys.stderr)
+                return False
+            finally:
                 signal.pthread_sigmask(signal.SIG_SETMASK, oldmask)
 
     def wait_for_active_job(last_task=None, backgrounded=False):
@@ -170,22 +180,14 @@ else:
         active_task = get_next_task()
         # Return when there are no foreground active task
         if active_task is None:
-            _give_terminal_to(_shell_pgrp)  # give terminal back to the shell
-            if backgrounded and hasattr(builtins, '__xonsh_shell__'):
-                # restoring sanity could probably be called whenever we return
-                # control to the shell. But it only seems to matter after a
-                # ^Z event. This *has* to be called after we give the terminal
-                # back to the shell.
-                builtins.__xonsh_shell__.shell.restore_tty_sanity()
             return last_task
-        pgrp = active_task.get('pgrp', None)
         obj = active_task['obj']
         backgrounded = False
-        # give the terminal over to the fg process
+        try:
-        if pgrp is not None:
+            _, wcode = os.waitpid(obj.pid, os.WUNTRACED)
-            _give_terminal_to(pgrp)
+        except ChildProcessError:  # No child processes
-        _continue(active_task)
+            return wait_for_active_job(last_task=active_task,
-        _, wcode = os.waitpid(obj.pid, os.WUNTRACED)
+                                       backgrounded=backgrounded)
         if os.WIFSTOPPED(wcode):
             print('^Z')
             active_task['status'] = "stopped"
@@ -307,9 +309,9 @@ def clean_jobs():
                     # newline
                     print()
                 print('xonsh: {}'.format(msg), file=sys.stderr)
-                print('-'*5, file=sys.stderr)
+                print('-' * 5, file=sys.stderr)
                 jobs([], stdout=sys.stderr)
-                print('-'*5, file=sys.stderr)
+                print('-' * 5, file=sys.stderr)
                 print('Type "exit" or press "ctrl-d" again to force quit.',
                       file=sys.stderr)
                 jobs_clean = False
@@ -354,31 +356,31 @@ def fg(args, stdin=None):
         return '', 'Cannot bring nonexistent job to foreground.\n'
 
     if len(args) == 0:
-        act = tasks[0]  # take the last manipulated task by default
+        tid = tasks[0]  # take the last manipulated task by default
     elif len(args) == 1:
         try:
             if args[0] == '+':  # take the last manipulated task
-                act = tasks[0]
+                tid = tasks[0]
             elif args[0] == '-':  # take the second to last manipulated task
-                act = tasks[1]
+                tid = tasks[1]
             else:
-                act = int(args[0])
+                tid = int(args[0])
         except (ValueError, IndexError):
             return '', 'Invalid job: {}\n'.format(args[0])
 
-        if act not in builtins.__xonsh_all_jobs__:
+        if tid not in builtins.__xonsh_all_jobs__:
             return '', 'Invalid job: {}\n'.format(args[0])
     else:
         return '', 'fg expects 0 or 1 arguments, not {}\n'.format(len(args))
 
     # Put this one on top of the queue
-    tasks.remove(act)
+    tasks.remove(tid)
-    tasks.appendleft(act)
+    tasks.appendleft(tid)
 
-    job = get_task(act)
+    job = get_task(tid)
     job['bg'] = False
     job['status'] = "running"
-    print_one_job(act)
+    print_one_job(tid)
     wait_for_active_job()
 
 

xonsh/proc.py

@@ -29,7 +29,7 @@ from xonsh.tools import (redirect_stdout, redirect_stderr, print_exception,
                          XonshCalledProcessError, findfirst, on_main_thread,
                          XonshError, format_std_prepost)
 from xonsh.lazyasd import lazyobject, LazyObject
-from xonsh.jobs import wait_for_active_job
+from xonsh.jobs import wait_for_active_job, give_terminal_to
 from xonsh.lazyimps import fcntl, termios, _winapi, msvcrt, winutils
 
 
@@ -1654,7 +1654,8 @@ class CommandPipeline:
 
     nonblocking = (io.BytesIO, NonBlockingFDReader, ConsoleParallelReader)
 
-    def __init__(self, specs, procs, starttime=None, captured=False):
+    def __init__(self, specs, procs, starttime=None, captured=False,
+                 term_pgid=None):
         """
         Parameters
         ----------
@@ -1695,6 +1696,7 @@ class CommandPipeline:
         self._closed_handle_cache = {}
         self.lines = []
         self._stderr_prefix = self._stderr_postfix = None
+        self._term_pgid = term_pgid
 
     def __repr__(self):
         s = self.__class__.__name__ + '('
@@ -1912,12 +1914,10 @@ class CommandPipeline:
     # Ending methods
     #
 
-    def end(self, tee_output=True):
+    def end_internal(self, tee_output):
         """Waits for the command to complete and then runs any closing and
         cleanup procedures that need to be run.
         """
-        if self.ended:
-            return
         if tee_output:
             for _ in self.tee_stdout():
                 pass
@@ -1932,6 +1932,21 @@ class CommandPipeline:
         self.ended = True
         self._raise_subproc_error()
 
+    def end(self, tee_output=True):
+        """
+        End the pipeline, return the controlling terminal if needed.
+
+        Main things done in end_internal().
+        """
+        if self.ended:
+            return
+        self.end_internal(tee_output=tee_output)
+        pgid = os.getpgid(0)
+        if self._term_pgid is None or pgid == self._term_pgid:
+            return
+        if give_terminal_to(pgid):  # if gave term succeed
+            self._term_pgid = pgid
+
     def _endtime(self):
         """Sets the closing timestamp if it hasn't been already."""
         if self.endtime is None: