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

Hitting ^D in an 'interact()' causes endless printing of 'exit' #49

Closed
aidanhs opened this Issue May 8, 2014 · 7 comments

Comments

Projects
None yet
3 participants
@aidanhs

aidanhs commented May 8, 2014

Run the following code:

import fdpexpect
import pty
import os
import sys

def proc_start(cmd_list):
        (child_pid, fd) = pty.fork()
        if child_pid == 0:
                # The first item of the list in the second argument is the name
                # of the new program
                try:
                        os.execvp(cmd_list[0], cmd_list)
                except OSError:
                        print "Failed to exec"
                        sys.exit(1)
        else:
                return fd

fd = proc_start(['/bin/bash'])
c = fdpexpect.fdspawn(fd)
c.interact()

You get a bash session, it works, you can do things.
Now, instead of hitting ^], hit ^D - observe the endless printing of 'exit' and your now locked-up terminal.

Going backwards through pypi versions, I can see this has definitely been broken since 3.0, but was working (it raised an exception instead of printing the same output forever) in 2.4

@takluyver

This comment has been minimized.

Member

takluyver commented May 8, 2014

I think this comment points to the issue:

# This is for Linux, which requires the blocking form
# of waitpid to # get status of a defunct process.
# This is super-lame. The flag_eof would have been set
# in read_nonblocking(), so this should be safe.

In interact, it's not using read_nonblocking(), so flag_eof is not sent, and it doesn't discover that the process has exited. If I'm right, fix coming up.

@takluyver

This comment has been minimized.

Member

takluyver commented May 8, 2014

Oh, no, it's simpler than that. You're using fdpexpect, so the fdspawn object doesn't know about the child process, only the file handle you've given it. The pty continues to exist after the process has died, so it has no way of knowing to stop reading.

Is there a reason you (or rather shutit) are replicating the functionality of pexpect? The easiest way to fix this would be to use the main spawn() class, which creates a pty, starts a new process in that pty, and tracks both the pty and the process. i.e. reduce that code sample to:

import pexpect
c = pexpect.spawn(['/bin/bash'])
c.interact()
@aidanhs

This comment has been minimized.

aidanhs commented May 8, 2014

Good point. I think this behaviour is probably historical, I'll need see if I can remember why I wrote it like that...
It may well be that just using plain pexpect is the best way to do this.

However, it doesn't change the fact that it terminates correctly in 2.4. This contradicts "[...] it has no way of knowing to stop reading."
Specifically, running on 2.4:

Traceback (most recent call last):
  File "test.py", line 21, in <module>
    c.interact()
  File "/usr/lib/python2.7/dist-packages/pexpect.py", line 1492, in interact
    self.__interact_copy(escape_character, input_filter, output_filter)
  File "/usr/lib/python2.7/dist-packages/pexpect.py", line 1520, in __interact_copy
    data = self.__interact_read(self.child_fd)
  File "/usr/lib/python2.7/dist-packages/pexpect.py", line 1510, in __interact_read
    return os.read(fd, 1000)
OSError: [Errno 5] Input/output error
@aidanhs

This comment has been minimized.

aidanhs commented May 8, 2014

Turns out the reason I did this initially was to connect pexpect to paramiko (which happens to expose a fd for the ssh terminal). I'm going to do it the way you suggested (thanks!) for shutit.

I'll leave this issue open, but I wouldn't call it a huge priority.
I'm not sure if it's possible to reproduce the issue for the original use case inside paramiko, but I do think it'd be preferable if pexpect have the behaviour it had before 3.0.

@takluyver

This comment has been minimized.

Member

takluyver commented May 9, 2014

Well, I wouldn't quite call throwing an error 'terminating correctly', but I take your point. Pexpect now catches that error, but it should probably stop looping when it does.

jquast added a commit that referenced this issue Jun 2, 2014

Resolves Issue #49 -- bad fdspawned-interact()
tested on centos using script by @aidanhs, not that doing your own
pty.fork() is really recommended if you're going to use interact

Otherwise, tested also using traditional use of interact, though
that worked fine previously -- it still works fine, now.  With
this, the test script provided by aidnhs now exits fine.

I would be interested in a test case and a more precise use-case.
@jquast

This comment has been minimized.

Member

jquast commented Jun 2, 2014

Yes, this bug was introduced by c7a180a -- @takluyver, see my pull request -- how does this match to what you intended with this comment?

@takluyver

This comment has been minimized.

Member

takluyver commented Jun 21, 2014

#72 is now merged, closing this.

@takluyver takluyver closed this Jun 21, 2014

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