Terminal input hidden after quiting subprocess started with `invoke.run` #303

Closed
tonysyu opened this Issue Jan 14, 2016 · 15 comments

Projects

None yet

5 participants

@tonysyu
tonysyu commented Jan 14, 2016

When I run invoke.run with a long-lived process and quit, terminal input becomes hidden after quitting. A simplified tasks.py example looks something like:

import invoke

@invoke.task
def run():
    invoke.run('python -m http.server')

and run using

$ invoke run

After quiting the server using Ctrl-C, keyboard inputs are no longer displayed. I can type all I want, but nothing is displayed until I hit enter, at which point, the terminal will display:

$-bash: random-letters-that-I-typed: command not found

Note that running the following does not result in the same issues.

import subprocess

subprocess.call('python -m http.server', shell=True)

System Info:

  • OSX 10.11.2
  • Python 3.4
  • Invoke 0.12
@bitprophet bitprophet added the Bug label Jan 20, 2016
@bitprophet bitprophet added this to the 0.12.1 milestone Jan 20, 2016
@bitprophet
Member

Thanks for the report - my guess is it's related to how we tweak terminal settings so we can read/mirror stdin, and perhaps we're not correctly undoing those changes in a try/except (or there's a bug in whatever we are doing there).

@presidento
Contributor

I ran also into it.

@bitprophet
Member

@tonysyu I can't replicate this on my end; when I Ctrl-C out of an Invoke-executed python -m http.server, I'm able to see what I type immediately afterwards, no problem.

(Side note: One does have to add -u to that call to see the Serving HTTP on 0.0.0.0 port 8000 ... prior to exiting; but I assume you knew that and omitted it for brevity :) Seems to have zero bearing on the issue, of course.)

My environment is Python 2.7 and/or 3.5, OS X 10.10, Terminal.app, zsh. Tested both the 0.12.0 tag and latest master (which does fix a handful of things, but none directly related to my guesswork above).

If you and/or @presidento can a) try latest master in case I did somehow fix it, and b) if not, share more details about your environment - that'd help. Thanks!

@bitprophet bitprophet removed this from the 0.12.1 milestone Feb 4, 2016
@noirbizarre

Hi !

I have exactly the same issue and I just tested with the latest master, it doesn't fix it for me.

Environment:
Python 2.7, ArchLinux, Bash version 4.3.43.
Tested with both 0.12.0 and latest master on Terminator and Guake.

@presidento
Contributor

I will test it tomorrow.

@presidento
Contributor

It works well on Windows 10 (64 bit) + Python 3.4 + Invoke 0.12.0

It fails on Ubuntu 15.10 (64 bit) + Python 3.5 + Invoke 0.12.0 and 0.12.1 + bash shell. Surprisingly it works with fish and zsh shells. (Unfortunately bash is the default for most Linux systems as I know.) I have tried with plain bash (without .bashrc) and it fails with it too.

An stty echo command helps to get back the input, but I don't know, why.

I hope it helps. Without fixing this we cannot upgrade.

@bitprophet
Member

Thanks for the details, sounds like perhaps a bash-ism or a Linux-ism, will try to replicate with this in mind soon. EDIT: yup, confirmed on bash 3.2.57 (OS X 10.10's default). Will see if I can figure out what bash and zsh are doing differently here.

@bitprophet
Member

As I suspected it's something to do with how we tickle the controlling TTY to be character buffered; as soon as I comment out these lines, normal terminal behavior resumes immediately after Ctrl-C.

Throwing some debug statements in shows that the finally in the try/finally is not executing, which is surprising, so I have to figure out if KeyboardInterrupt somehow routes around those, or if it's a threading termination issue (seems likely, since character_buffered is used in handle_stdin which forms the body of the stdin read/write thread, and I guess I shouldn't necessarily expect thread shutdown to be considered "an exception"?)

Implies that if this is happening in all situations, zsh is recovering from the lack of terminal settings restoration better than bash does, or the settings change (setcbreak) isn't actually doing anything on zsh to begin with. (This code is ported from Fabric 1 so it's a bit old and I don't remember the gritty details.)

@bitprophet
Member

Yup, for reasons not well documented, we set the IO worker threads to daemon mode, meaning as soon as the main thread exits, the overall Python process exits, because all that's left are daemon threads.

Feels like what's necessary is to add a try/except in the spot where we wait for the subprocess, structured such that we're able to signal & tie off the threads prior to reraising the initial exception. This seems like maybe a good idea regardless of whether or not we continue using daemon threads.

(I think I still lean towards 'daemon good' insofar as it means failure cases default to "everything exits" instead of "something hangs forever"...)

@bitprophet
Member

In trying to test this, I'm finding more evidence that we do need stronger controls on these threads & their lifetime vs the main thread - e.g. if one does, effectively:

try:
    run('python -m http.server')
except KeyboardInterrupt:
    pass
more_things_here()

If you Ctrl-C the server subprocess, once your code is at/past in more_things_here land, the IO threads spun up by the run are still running, doing goodness knows what. Granted, in most real cases, they'd quickly bomb out on their own, trying to read from or write to the now broken pipes to the subprocess. (In the test case, those pipes are dummy IO objects which are happily sticking around accepting method calls.)

Still though, it shows the need for Runner to clean up after itself better. Poking.

@bitprophet bitprophet added a commit that referenced this issue Feb 8, 2016
@bitprophet bitprophet Test & fix re #303 a40c11c
@bitprophet bitprophet added a commit that referenced this issue Feb 8, 2016
@bitprophet bitprophet Changelog re #303 6042d99
@bitprophet
Member

Got it.

@bitprophet bitprophet closed this Feb 8, 2016
@bitprophet bitprophet added this to the 0.12.2 milestone Feb 8, 2016
@thedrow
Contributor
thedrow commented Feb 8, 2016

@bitprophet Can you please release a 0.12.1.1? This is really annoying.

@presidento
Contributor

Tested on Ubuntu with Bash, it works well with v0.12.2, thank you for the fix!

@bitprophet
Member

@thedrow 0.12.2 is out.

@thedrow
Contributor
thedrow commented Feb 8, 2016

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment