Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

shell command hangs if it expects input #514

Closed
satra opened this issue Jun 10, 2011 · 36 comments
Closed

shell command hangs if it expects input #514

satra opened this issue Jun 10, 2011 · 36 comments

Comments

@satra
Copy link
Contributor

satra commented Jun 10, 2011

this simply hangs unless i hit control-C

In [1]: rm crash-20110606-112922-satra-segstats.npz
remove crash-20110606-112922-satra-segstats.npz? ^C

this works fine.

In [1]: !rm crash-20110606-112922-satra-segstats.npz

affects any kernel frontend (qtconsole, notebook, etc.)

@minrk
Copy link
Member

minrk commented Jun 10, 2011

shell escapes from the qtconsole don't forward input to the subprocess, so anything that expects input will hang.

Either we need to open the subprocesses in a manner that they know they can't expect input, or we need to hook up console input to the remote pexpect process.

I don't actually know how to do either one, but I think the former is more likely to be doable.

@fperez
Copy link
Member

fperez commented Jun 10, 2011

Yes, this is nasty and worries me, and I don't know how to do either. Satra, you'd need to always do rm -f from the cmd line to ensure it doesn't wait for input.

Min, the problem is that we use pexpect precisely so the qt console works like a terminal, but that tricks processes into thinking they're running in a real terminal. I don't know how to trick them into thinking 'this is a terminal for output, but it has no input at all, so work as in batch mode as far as input is concerned'. I don't actually know if it's possible within pexpect at all.

@takluyver
Copy link
Member

(Tagged as high priority on the basis that it's a serious issue, but we might not block 0.11 on it. Feel free to disagree)

@minrk
Copy link
Member

minrk commented Jun 10, 2011

it is high priority, but I think we can't block on it. What we might block 0.11 on is some kind of message, because it's confusing and frustrating to block for IO on simple operations.

@minrk
Copy link
Member

minrk commented Jun 10, 2011

pexpect seems to be designed for interacting with a subprocess, which we specifically don't want to do.
What is the advantage of using pexpect instead over subprocess? If you pass devnull as stdin to subprocess, processes don't seem to expect input.

@takluyver
Copy link
Member

But there are situations like ls which, when they detect they're running in an interactive terminal, produce nicer, more human-readable output (e.g. adding colours). Quite what mechanisms are used to do that, I don't know. Maybe we should just define aliases for a few such cases, and let the rest run 'raw'.

@fperez
Copy link
Member

fperez commented Jun 10, 2011

On Fri, Jun 10, 2011 at 3:06 PM, minrk
reply@reply.github.com
wrote:

What is the advantage of using pexpect instead over subprocess?  If you pass devnull as stdin to subprocess, processes don't seem to expect input.

But then you get output from things like ls that's not nicely
formatted. The 'console experience' goes quickly to the toilet for
many common tasks if system utilities think they're talking to a pipe
instead of a terminal.

That was the reason for going pexpect.

Cheers,

f

@fperez
Copy link
Member

fperez commented Jun 10, 2011

On Fri, Jun 10, 2011 at 3:20 PM, takluyver
reply@reply.github.com
wrote:

But there are situations like ls which, when they detect they're running in an interactive terminal, produce nicer, more human-readable output (e.g. adding colours). Quite what mechanisms are used to do that, I don't know. Maybe we should just define aliases for a few such cases, and let the rest run 'raw'.

That's what I meant...

I have no idea how to make an alias for ls that will make it think
it's in a terminal, as best I know that's all auto-detected internally
by ls (I just read its TeXinfo docs and the manpage, and there's no
mention of a way to achieve what you propose).

@minrk
Copy link
Member

minrk commented Jun 10, 2011

Ah, okay. As long as there is a reason to use pexpect, that's fine.

For 0.11, all I think we need is a disclaimer that says 'WARNING: Don't launch shell processes that expect input, because they won't work!' somewhere.

It's actually not precisely correct that they don't work at all, they just don't work in any reasonable way - I think they do respond to stdin from the launching terminal window. Not that that really counts for anything.

@takluyver
Copy link
Member

I tried y, <return> at the terminal window when rm file wanted input, but it didn't have any effect.

@minrk
Copy link
Member

minrk commented Jun 11, 2011

Okay - I could swear that worked for some command, but it doesn't really matter. Either commands need to get input from the console window or they need to know that they will never receive input.

@minrk
Copy link
Member

minrk commented Nov 10, 2011

A question: since we don't accept passing input to the subprocess, under what circumstances is using pexpect as we do better than just using subprocess.Popen?

A further issue that prompts promotion of this issue to critical, to ensure we get some resolution: The command string our piped system constructs (see here ) is simply wrong. If there's a double-quote anywhere in the command, it will get parsed incorrectly.

A simple example that fails in the qtconsole:

In [1]: !python -c "import sys"
  File "<string>", line 1
    import
         ^
SyntaxError: invalid syntax

Because the command being executed is actually sh -c "python -c "import sys"", which is obviously wrong. I think this would not occur if we were using Popen with shell=True, which should be equivalent.

@takluyver
Copy link
Member

The rationale for using pexpect is that some things (like ls, wget) produce human readable output if they believe they're running in a terminal, rather than sending output to a pipe.

@minrk
Copy link
Member

minrk commented Nov 10, 2011

Right, I remember that now - we get colors, etc. when the subprocess thinks it is in a terminal. That makes perfect sense, but if we can't differentiate between telling the subprocess that we should get pretty colors / column formatting, and telling the subprocess that it can expect input that it will never receive, maybe it's not worth it.

I've fixed the issue that prompted me to upgrade this to critical (PR #989), so I brought this one back down to high.

We should probably still try to instruct subprocesses that they can't get any input. It's possible that just writing EOF to the subprocess would do it in simple cases.

@takluyver
Copy link
Member

There are other things, like wget doesn't appear to produce any output at all if it's going to a pipe (although that's not what the manual suggests). We should probably try to work out how many things change their output, and how many expect input when running in a terminal. Unless of course we can find a workaround like EOF.

@minrk
Copy link
Member

minrk commented Nov 10, 2011

I found that sending eof does indeed prevent hanging, but it behaves (logically enough) just like if you pressed ^D in an actual terminal, which is not always sane, and the hang will not be prevented in subprocesses that handle EOF as anything other than exit.

For instance, !rm -i foo will simply output remove foo? ^D��, and can never actually remove the file.

I don't suppose there is a way to detect that the subprocess is trying to read stdin, is there? Because if so, it would be trivial for that to trigger raw_input, which is what should really happen anyway.

@fperez
Copy link
Member

fperez commented Nov 10, 2011

On Thu, Nov 10, 2011 at 1:59 PM, Min RK
reply@reply.github.com
wrote:

I don't suppose there is a way to detect that the subprocess is trying to read stdin, is there?

Maybe by using the file descriptor duplication tricks that R. Kern
posted on the mailing list a few days ago, I'm not sure.
Alternatively, it might be possible to pass to the subprocess call an
object whose .read() method calls back our raw_input, but I have no
idea if that will work or not.

@takluyver
Copy link
Member

Is there some way we could arrange for any keystrokes at the terminal while
a subprocess is running to be forwarded to the subprocess?

@minrk
Copy link
Member

minrk commented Nov 10, 2011

I've been thinking about that - it seems like it would need to rearrange how we do stdin forwarding a bit, because right now our stdin model matches raw_input directly - an explicit call in the kernel makes a request of the frontend, and expects a reply.

Blind stdin-forwarding, on the other hand, is better suited to a one-way PUB/SUB model, where keystrokes are just sent, and the kernel doesn't actually know anything about whether the subprocess or the frontend thinks stdin should be happening.

@fperez
Copy link
Member

fperez commented Nov 10, 2011

that's why I referred to @rkern's tricks, they might help. This is definitely a pretty nasty issue...

@mwiebe
Copy link
Contributor

mwiebe commented Jan 25, 2012

The way I'm imagining things, it would be nice if the subprocess was interactive like it was embedded in a cmd or xterm window, instead of a batch command. The I in IPython stands for interactive, right? ;)

@minrk
Copy link
Member

minrk commented Jan 25, 2012

Yes, that would definitely be nice. If you want to take a stab at making it work, pull requests are of welcome.

This is the difficulty of not having a real terminal. Some things (display, multiline editing, etc.) are much better with the message-based multiprocess design than a plain terminal, but some simple terminal things (e.g. subprocesses with full interactivity) suddenly become weird and complicated.

@mwiebe
Copy link
Contributor

mwiebe commented Jan 25, 2012

Is there any document which describes best practices for how to dip ones toe into ipython development? Something which gives a tour of where all the files are, how the architecture works, what environment the core developers use is, etc?

@takluyver
Copy link
Member

@m-paradox : There's a fairly substantial developer guide, though we don't currently have a brief summary.

For this, I think the files you'd need to look at are in IPython.zmq (the messaging architecture), the process modules in IPython.utils and the Qt console code in IPython.frontend.

@whitelynx
Copy link

I've been thinking about that - it seems like it would need to rearrange how we do stdin forwarding a bit, because right now our stdin model matches raw_input directly - an explicit call in the kernel makes a request of the frontend, and expects a reply.

Blind stdin-forwarding, on the other hand, is better suited to a one-way PUB/SUB model, where keystrokes are just sent, and the kernel doesn't actually know anything about whether the subprocess or the frontend thinks stdin should be happening.

Would it be possible to just create another explicit call (say, get_input) that works similarly to the one that triggers raw_input, but would (if the input buffer contains data) return the contents of the input buffer and clear it or (if the input buffer is empty) wait until a keypress is detected, and then send it? The only other thing I see that would have to happen there is to have the ability to interrupt the last get_input call's keypress wait when the subprocess ends... other than that, it seems to me that would fix the issue.

If this approach seems viable, I may be able to hack out a solution in the next couple of weeks, but I'm pretty busy with work and other projects, so if someone else wants to tackle it, feel free.

@whitelynx
Copy link

Or, alternately, would it be possible to just have enable_keystroke_forwarding and disable_keystroke_forwarding messages that could be sent to the frontend, which would tell the frontend to send keystroke messages to the backend?

@ellisonbg
Copy link
Member

@takluyver and @minrk what do you think we should do with this one? It has been around for a really long time and is targeted to 2.0.

@minrk
Copy link
Member

minrk commented Jan 26, 2014

It's not going to be fixed for 2.0, but there are approaches we can take to address it in the future (at the very least, we should get to the point of exceptions instead of hangs). I'd mark it as wishlist.

@NickSto
Copy link

NickSto commented Feb 17, 2014

Just ran into this myself. The weirdest part is that the default aliases seem to use the -i flag for cp, mv, and rm. Unless there is some reason these were kept (even though they hang the session), a quick mitigation step would be to just remove these aliases?

@sychan
Copy link
Contributor

sychan commented Apr 24, 2014

We ran into this as well. An easy mitigation, which I just implemented in our standard deployment, is to modify the ipython_notebook_config.py file to set c.AliasesManager.user_aliases with some new aliases that override the cp,mv and rm aliases. This only effects the notebook and leaves the CLI in its normal state.

@takluyver
Copy link
Member

Those aliases should probably be updated if someone cares to make a PR.

@sychan
Copy link
Contributor

sychan commented Apr 24, 2014

Should they be updated only for the notebook config, or should they be updated in general?

@takluyver
Copy link
Member

I'd be OK with just updating them in general - I don't see an easy mechanism for the defaults to be different for the ZMQ frontends, and I'd expect our aliases to work like bash commands in any case.

@rossant
Copy link
Contributor

rossant commented Jun 3, 2014

A command like conda install something makes the kernel hang indefinitely, requiring a kernel restart, because that command excepts a y/n user input. Will that be fixed eventually?

@minrk
Copy link
Member

minrk commented Jun 3, 2014

Someday.

@Carreau
Copy link
Member

Carreau commented Jul 21, 2016

Hi,

The qtconsole has been moved to https://github.com/jupyter/qtconsole, and this issue had no activity for quite some time. If this is still relevant I would suggest this issue to be reopen on the above cited repository.

This will allow us to keep the number of IPython issues smaller and focussed on IPython. As currently there is an extremely high number of opened issues , it make it really hard to find if a problem is actually known, or going to be worked one.

Feel free to reopen if need, for the time being I'm going to close this.

Thanks.

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

Successfully merging a pull request may close this issue.