This issue tracker has been migrated to GitHub ,
and is currently read-only.
For more information,
see the GitHub FAQs in the Python's Developer Guide.
Created on 2016年12月23日 14:17 by Cornelius Diekmann, last changed 2022年04月11日 14:58 by admin.
| Files | ||||
|---|---|---|---|---|
| File name | Uploaded | Description | Edit | |
| pty.patch | Cornelius Diekmann, 2016年12月23日 14:17 | Hacky patch with placeholder | ||
| pty.patch | Cornelius Diekmann, 2016年12月23日 19:40 | Patch with test cases. If the tests are not red on other obscure UNIXes, consider merging. | review | |
| pty.patch | Cornelius Diekmann, 2016年12月23日 22:30 | Same, but without git patch header | review | |
| pty.patch | Cornelius Diekmann, 2016年12月23日 22:37 | Same, but without git patch header. Why does the review tool not show test_pty.py? | review | |
| pty_and_tests.patch | Cornelius Diekmann, 2016年12月23日 23:35 | Same, but less broken diff format. | review | |
| test_pty_and_doc.patch | Cornelius Diekmann, 2016年12月24日 13:59 | Document and test existing behavior, no change in behavior. | review | |
| Messages (9) | |||
|---|---|---|---|
| msg283880 - (view) | Author: Cornelius Diekmann (Cornelius Diekmann) * | Date: 2016年12月23日 14:17 | |
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.
|
|||
| msg283899 - (view) | Author: Cornelius Diekmann (Cornelius Diekmann) * | Date: 2016年12月23日 19:40 | |
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). |
|||
| msg283904 - (view) | Author: Cornelius Diekmann (Cornelius Diekmann) * | Date: 2016年12月23日 22:30 | |
Removed git patch header from pty.patch to make python code review tool happy. Sorry, this is my first contribution. |
|||
| msg283905 - (view) | Author: Cornelius Diekmann (Cornelius Diekmann) * | Date: 2016年12月23日 22:37 | |
Review tool still did not show the test_pty.py file. Sry. |
|||
| msg283909 - (view) | Author: Cornelius Diekmann (Cornelius Diekmann) * | Date: 2016年12月23日 23:35 | |
Make review tool happy by giving it less broken patch format :) `make patchcheck` is already happy. Sorry for the noise :( |
|||
| msg283912 - (view) | Author: Martin Panter (martin.panter) * (Python committer) | Date: 2016年12月24日 01:25 | |
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 :)
|
|||
| msg283952 - (view) | Author: Cornelius Diekmann (Cornelius Diekmann) * | Date: 2016年12月24日 13:59 | |
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! |
|||
| msg284494 - (view) | Author: Cornelius Diekmann (Cornelius Diekmann) * | Date: 2017年01月02日 18:43 | |
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. |
|||
| msg284816 - (view) | Author: Cornelius Diekmann (Cornelius Diekmann) * | Date: 2017年01月06日 12:37 | |
[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 |
|||
| History | |||
|---|---|---|---|
| Date | User | Action | Args |
| 2022年04月11日 14:58:41 | admin | set | github: 73240 |
| 2017年01月06日 12:37:37 | Cornelius Diekmann | set | messages: + msg284816 |
| 2017年01月03日 12:24:01 | vstinner | set | nosy:
+ vstinner |
| 2017年01月02日 18:43:10 | Cornelius Diekmann | set | messages: + msg284494 |
| 2016年12月24日 13:59:24 | Cornelius Diekmann | set | files:
+ test_pty_and_doc.patch messages: + msg283952 |
| 2016年12月24日 01:25:23 | martin.panter | set | messages:
+ msg283912 versions: - Python 3.4, Python 3.5, Python 3.6 |
| 2016年12月23日 23:35:28 | Cornelius Diekmann | set | files:
+ pty_and_tests.patch messages: + msg283909 |
| 2016年12月23日 22:37:13 | Cornelius Diekmann | set | files:
+ pty.patch messages: + msg283905 |
| 2016年12月23日 22:30:43 | Cornelius Diekmann | set | files:
+ pty.patch messages: + msg283904 |
| 2016年12月23日 19:40:39 | Cornelius Diekmann | set | files:
+ pty.patch messages: + msg283899 versions: + Python 3.6, Python 3.7 |
| 2016年12月23日 14:17:17 | Cornelius Diekmann | create | |