My OS: Debian GNU/Linux 8.6 (jessie)
Python 3.4.2
pty.py from Python-3.5.2/Lib (pty.py is just a tiny, portable python file which did not see many changes)


Bug Report

Steps to Reproduce:

I wrote a very simple python remote shell:

#!/usr/bin/env python3
import pty
pty.spawn('/bin/sh')


It can be run in a terminal (call it TermA) with `nc -e ./myfancypythonremoteshell.py -l -p 6699`
In a different terminal (call it TermB), I connect to my fancy remote shell with `nc 127.0.0.1 6699`.
The shell works fine. In TermB, I quit by pressing ctrl+c.

Observed Behavior: In TermA, the nc process, the python process, and the spawned /bin/sh still exist. They still occupy TermA.

Expected Behavior: The client in TermB has disconnected, /bin/sh in TermA can no longer read anything from stdin and it should close down. Ultimately, in TermA, nc should have exited successfully.

Fix: End the _copy loop in pty.py once EOF in STDIN is reached. Everything shuts itself down automatically. I included a small patch to demonstrate this behavior.

This patch is not meant to go straight into production. I have not verified if this behavior somehow breaks other use cases. This bug report is meant to document exactly one specific use case and supply exactly one line of code change for it.

This issue is related to issue26228. Actually, it is complementary. issue26228 wants to return if master_fd is EOF, this issue wants to return if stdin is EOF. Both behaviors together looks good to me. By the way, I hacked a hacky `assert False` into my patch as a placeholder for issue26228's proper handling of exec failures at that part of the code.

I suggest to combine the patches of this issue and issue26228.

I wrote a proper patch for the issue of handling EOF in STDIN, including tests. My patch is against the github mirror head, but don't worry, the files I touch haven't been touched in recent years ;-)

I only tested on Linux. My patch only addresses the issue in this thread. It does not include the patch for issue26228. I still recommend to also merge the patch for issue26228 (but I don't have a FreeBSD box to test).

Removed git patch header from pty.patch to make python code review tool happy. Sorry, this is my first contribution.

Review tool still did not show the test_pty.py file. Sry.

Make review tool happy by giving it less broken patch format :)
`make patchcheck` is already happy.
Sorry for the noise :(

This is a change in behaviour of the _copy() loop: it will stop as soon as EOF is read from the parent’s input, and immediately close the terminal master. Unpatched, the loop continues to read output from the child, until the child closes the terminal slave.

I agree that your new behaviour may be desired in some cases, but you need to respect backwards compatibility. With your patch, things will no longer work robustly when the child “has the last word”, i.e. it writes output and exits without reading any (extra) input. Simple example: the child prints a message, but the parent has no input:

python -c 'import pty; pty.spawn("./message.py")' < /dev/null

Any new functionality would also need documenting. (If you want to suggest some wording to document the existing behaviour better, that would also be welcome :)

Thank you Martin very much. To resolve this issue, I decided to document the current behavior and add test cases for it. No change in behavior is introduced. This hopefully allows to close this issue. The test cases for the current behavior ensure that we can (at some point in the future) add some different behavior without breaking backwards compatibility.

Fixed: Observed behavior is now expected+documented behavior. Improved test cases.

Happy Holidays!

Status change: I proposed a generic test suite for pty.spawn() in issue29070. Once we have agreed on the current behavior of pty.spawn() and the test suite is merged, I would like to come back to this issue which requests for a change in behavior of pty.spawn(). Currently, I would like to document that this issue is waiting for issue29070 and this issue doesn't need any attention.

[no status change, this issue currently does NOT need any attention]

To keep issues separate, I just wanted to document a comment about this issue mentioned in issue29070. It refers to the _copy loop.

        if STDIN_FILENO in rfds:
             data = stdin_read(STDIN_FILENO)
             if not data:
                 fds.remove(STDIN_FILENO)
+                # Proposal for future behavior change: Signal EOF to
+                # slave if STDIN of master is gone. Solves issue29054.
+                # os.write(master_fd, b'\x04')
             else:
                 _writen(master_fd, data)

> vadmium 2017/01/04 21:50:26
> I suggest leaving this for the other [issue29054, i.e. this] bug. Another option may be to send SIGHUP
> (though I am far from an expert on Unix terminals :).

http://bugs.python.org/review/29070/diff/19626/Lib/pty.py