Interrupting a running task with Ctrl+C

[pmdarrow]

Interrupting a running task with Ctrl+C results in this traceback:

^CTraceback (most recent call last):
  File "/Users/peter/Envs/are/bin/inv", line 9, in <module>
    load_entry_point('invoke==0.8.2', 'console_scripts', 'inv')()
  File "/Users/peter/Envs/are/lib/python2.7/site-packages/invoke/cli.py", line 295, in main
    dispatch(sys.argv)
  File "/Users/peter/Envs/are/lib/python2.7/site-packages/invoke/cli.py", line 288, in dispatch
    return executor.execute(*tasks, dedupe=dedupe)
  File "/Users/peter/Envs/are/lib/python2.7/site-packages/invoke/executor.py", line 89, in execute
    task=task, name=name, args=args, kwargs=kwargs
  File "/Users/peter/Envs/are/lib/python2.7/site-packages/invoke/executor.py", line 128, in _execute
    return task(*args, **kwargs)
  File "/Users/peter/Envs/are/lib/python2.7/site-packages/invoke/tasks.py", line 108, in __call__
    result = self.body(*args, **kwargs)
  File "/Users/peter/Code/are-platform/API/tasks.py", line 56, in start
    run_env('python are/servers/dev_server.py', shell_env, verbose)
  File "/Users/peter/Code/are-platform/API/tasks.py", line 27, in run_env
    run('%s %s' % (env_vars, cmd))
  File "/Users/peter/Envs/are/lib/python2.7/site-packages/invoke/runner.py", line 143, in run
    stdout, stderr, exited, exception = func(command, warn, hide)
  File "/Users/peter/Envs/are/lib/python2.7/site-packages/invoke/runner.py", line 65, in run
    stdout, stderr = process.communicate()
  File "/usr/local/Cellar/python/2.7.6/Frameworks/Python.framework/Versions/2.7/lib/python2.7/subprocess.py", line 798, in communicate
    return self._communicate(input)
  File "/Users/peter/Envs/are/lib/python2.7/site-packages/invoke/monkey.py", line 58, in _communicate
    rlist, wlist, xlist = select.select(read_set, write_set, [])
KeyboardInterrupt

I'm migrating from Fabric which handles SIGINT nicely by exiting with sys.exit(1) and printing Stopped.. It also properly forwards the interrupt to any processes started with fabric.api.local(). How can I get this behaviour in Invoke?

Willing to contribute a fix for this if I'm pointed in the right direction!

[sophacles]

I'm 👍 on this

[presidento]

I'm also willing to contribute for this if I'm pointed in the right direction.

[frol]

@presidento Just looking at the code, I would say that the place where handling of KeyboardInterrupt exception should be added is here.

[presidento]

@frol I don't think so. IMHO, the best way to handling Ctrt+C would be sending the SIGKILL signal to the external process and wait gracefully until it stops. Now the behaviour is very different (#315): Invoke exits, while the started process remain running in the background.

Also I don't want to Invoke handle the Ctrl+C, because we frequently use invoke run bash. And I run other commands within the bash shell, so it would be good to return into it after pressing Ctrl+C.

[frol]

@presidento I see your point. Then it seems it should be also patched here. Catch KeyboardInterrupt exception, send SIGTERM to a child (why would you want to kill it with SIGKILL?), wait again and once it is terminated, I would raise KeyboardInterrupt to exit since this is what I would expect as a user instead of continuing running. (Also, you may consider handling subsequent CTRL+C hits, which will kill a misbehaved child with SIGKILL signal.)

[bitprophet]

My off the cuff feedback:

• Us not submitting the Ctrl-C "signal" downstream to the subprocess is an oversight and needs to happen. The general design goal in Invoke's runner functionality is "as transparent as we can get without sacrificing our own feature set" :)
  • Honestly, we should replicate most if not all signals we get, to the running subprocess, if we receive signals during the "waiting for subprocess" phase of our execution.
• Again, just off the cuff, I think the "best" place to 'mirror' the KeyboardInterrupt downstream may be here - the recent changes fixing Terminal input hidden after quiting subprocess started with invoke.run #303 added that try/except mostly to nab KeyboardExceptions. So within the except, or somewhere soon afterwards, is probably where we ought to signal the subprocess.
  • Likely needs to be implemented as an abstract Runner method + explicit Local method because the act of signaling differs between local and remote execution.
  • I also agree that SIGTERM is probably the best default to go with, though making this configurable might be a good idea...

[bitprophet]

Poking at this myself now, referencing #331 as a jumping-off point (thanks @presidento!). Notes as I go:

• Started with tests & docs :)
• Put the basic logic around "do things with the exception encountered during wait" inside run instead of wait itself, as it's implementation-agnostic & also just felt cleaner.
  • Then added signal_interrupt() as the implementation-specific subroutine that handles signaling
• Preserved @presidento's use of killpg instead of kill since that does feel like the most correct call (more likely to correctly handle situations where the subprocess spawns its own subprocesses)
• Handled the situations where pty=False (which also includes some Windows considerations)

EDIT: getting permission denied errors on killpg; digging into that next.

[bitprophet]

Seems like the killpg thing (I get OSError: permission denied) is a Darwin/BSD problem (e.g. this); but it works OK on Linux.

That said, regular kill seems to work OK on both platforms, and while submitting the signal to the whole process group sounds nice, I'm not sure whether it's a big enough benefit to be worth the apparent cross-platform issues. E.g. I'd expect most subprocesses that spawn their own subprocesses to handle a single SIGINT sent to just-them, gracefully enough.

So I'm probably going to go with kill for now, tho as usual I'm open to arguments. (Just, the argument needs to overcome the platform issue or the complexity cost of working around it :))

[presidento]

Using killpg was not conscious, I just googled for an existing problem and have found a "works for me" solution (for Linux). So I don't have arguments againts kill.

[bitprophet]

Great, glad to hear it! :)