homepage

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.

classification
Title: IDLE - regression with exit() and quit()
Type: behavior Stage: resolved
Components: IDLE Versions: Python 3.3, Python 3.4, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: roger.serwy Nosy List: benjamin.peterson, georg.brandl, larry, pitrou, python-dev, roger.serwy, serhiy.storchaka, terry.reedy
Priority: release blocker Keywords: patch

Created on 2013年03月31日 07:45 by roger.serwy, last changed 2022年04月11日 14:57 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
fileno_close.patch roger.serwy, 2013年03月31日 07:44 review
issue17585.patch roger.serwy, 2013年03月31日 17:41 review
issue17585_2.7.patch roger.serwy, 2013年03月31日 18:26 review
site_reversion.patch roger.serwy, 2013年03月31日 19:26 review
Messages (17)
msg185619 - (view) Author: Roger Serwy (roger.serwy) * (Python committer) Date: 2013年03月31日 07:44
This issue is a split from #5492, where Terry noticed a serious regression that "quit()" and "exit()" no longer work in IDLE.
Before #9290, the PyShell object itself was stdin and it didn't have a "fileno" method. The code in site.py would skip over the call to fileno, leaving fd == -1, which then called close() on stdin, effectively the close() method of PyShell. 
The application of #9290 introduced PseudoFile as a subclass of io.TextIOBase which has a "fileno" method. The site.py code find it, calls it, but an error gets raised so that the close() method doesn't get called. Even if you subclass fileno(), you still need to subclass close() so that it calls the original close function in PyShell.
The attached patch, fileno_close.patch, fixes that issue. I would argue that this patch should be applied to all the release candidates as it corrects a serious regression.
I would like to hear your thoughts before applying the patch.
msg185620 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2013年03月31日 08:07
I thought of this exact patch, but then decided against it.
1. The bug is in site.py -- see thread
[Python-Dev] Idle, site.py, and the release candidates
for a better explanation and solution.
2. The Idle behavior, inherited from io.IOBase is correct.
3. The consequence on *other* Python code of a fake fileno is unknown.
The solution I gave for site.py is to begin .__call__ with
if sys.stdin.__name__ == 'PseudoInputFile': sys.stdin.close()
This should be safe to do without another release candidate.
The name as seen by the user process should be checked but I cannot do so at the moment (on substitute substitute computer). This would only be done in the release by the file managers. The exact decision is up to them.
So I would replace IDLE with site.py in the title.
msg185621 - (view) Author: Roger Serwy (roger.serwy) * (Python committer) Date: 2013年03月31日 08:27
I agree that site.py's Quitter exception logic has a flaw as described on the email from python-dev. But I disagree that the problem of IDLE not exiting is due to site.py. Even if you fix site.py (which I did), calling sys.stdin.close() won't close IDLE since PseudoInputFile's close method doesn't call PyShell's close method.
msg185628 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2013年03月31日 15:39
I see that now. Then both files should be fixed. I still object to introducing a buggy .fileno ;-).
msg185633 - (view) Author: Roger Serwy (roger.serwy) * (Python committer) Date: 2013年03月31日 17:41
Terry, I agree that the original patch introduces a buggy fileno and it "knows" too much about the internals of site.py which could change.
The attached patch modifies site.py, similar to how Nick Coghlan split the two function calls in a single try/except block into two separate try/except blocks. I tried to keep the change as small as possible to avoid introducing any unintended regressions. The change does pass the test suite on 3.4 on Linux.
msg185635 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013年03月31日 18:14
Please read http://mail.python.org/pipermail/python-dev/2013-March/125021.html . The fileno() check shouldn't be needed anymore.
Also, it can't effect 2.7, or you have another problem.
msg185636 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013年03月31日 18:15
(s/effect/affect/, sorry)
msg185637 - (view) Author: Roger Serwy (roger.serwy) * (Python committer) Date: 2013年03月31日 18:26
Antoine, it does affect 2.7 since sys.stdin.close() doesn't call PyShell's close method. #9290 introduced this regression. I accept some responsibility because my manual testing of those patches in #9290 didn't include trying the exit() and quit() methods.
The attached patch fixes the problem on the 2.7 branch.
msg185638 - (view) Author: Roger Serwy (roger.serwy) * (Python committer) Date: 2013年03月31日 18:36
I would also argue that the fileno_close.patch ought to be applied to the 3.2.4 and 3.3.1 release candidates. The changes in #9290 were applied only two months ago.
msg185640 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013年03月31日 18:39
I won't argue about any changes in IDLE, but the fileno-checking code in site.py should probably be removed, not complicated.
msg185642 - (view) Author: Roger Serwy (roger.serwy) * (Python committer) Date: 2013年03月31日 19:26
I agree with Antoine that fileno checking code should be removed. I dug deeper into why that was introduced in the first place.
00b136b7da84 introduced the warning "Trying to close unclosable fd!"
86358b43c8bb intoduced the change to site.py about not closing stdin if wrapping fd 0 to avoid the warning.
23d9cb777d9a removed the warning as part of issue4233.
So the code in site.py for checking fileno is cruft and can be safely removed. The site_reversion patch backs out of 86358b43c8bb.
msg186182 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2013年04月07日 07:46
Perhaps PseudoInputFile.close() should call super().close() to set closed flag. Perhaps close() should be even implemented in PseudoFile.
It's my fault in this regression. I deliberately have not implemented PseudoFile.close(), because I saw no sense in deliberate calling of sys.std*.close() and considered closing the window with unpremeditated closing of the standard stream is too dangerous.
msg186545 - (view) Author: Roger Serwy (roger.serwy) * (Python committer) Date: 2013年04月11日 04:32
Serhiy, don't worry. There's still plenty of broken found in IDLE.
Antoine, may I modify site.py with site_reversion.patch? If not, then I can include a workaround in IDLE.
msg186548 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013年04月11日 06:29
Yes, fixing site.py is fine.
msg186602 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2013年04月12日 00:24
New changeset 99e363e67844 by Roger Serwy in branch '2.7':
#17585: Fixed IDLE regression. Now closes when using exit() or quit().
http://hg.python.org/cpython/rev/99e363e67844
New changeset d3c67e2fc68c by Roger Serwy in branch '3.3':
#17585: Fixed IDLE regression. Now closes when using exit() or quit().
http://hg.python.org/cpython/rev/d3c67e2fc68c
New changeset 82451c88b3c0 by Roger Serwy in branch 'default':
#17585: merge with 3.3.
http://hg.python.org/cpython/rev/82451c88b3c0 
msg186603 - (view) Author: Roger Serwy (roger.serwy) * (Python committer) Date: 2013年04月12日 00:29
Thanks, Antoine.
I am closing this issue as fixed.
msg188177 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2013年04月30日 19:12
This fix introduces other bug (see issue17838). I think this fix with issue17838 fix should be included in next regression releases.
History
Date User Action Args
2022年04月11日 14:57:43adminsetgithub: 61785
2013年05月12日 09:42:35georg.brandlsetstatus: open -> closed
2013年05月12日 09:40:51georg.brandlsetversions: - Python 3.2
2013年04月30日 19:12:13serhiy.storchakasetstatus: closed -> open
priority: critical -> release blocker
versions: + Python 3.2
nosy: + benjamin.peterson, georg.brandl, larry

messages: + msg188177
2013年04月23日 01:10:01roger.serwylinkissue17817 superseder
2013年04月12日 00:29:42roger.serwysetstatus: open -> closed
resolution: fixed
messages: + msg186603

stage: patch review -> resolved
2013年04月12日 00:24:18python-devsetnosy: + python-dev
messages: + msg186602
2013年04月11日 06:29:14pitrousetmessages: + msg186548
2013年04月11日 04:32:44roger.serwysetmessages: + msg186545
2013年04月07日 07:46:02serhiy.storchakasetmessages: + msg186182
2013年04月07日 07:10:20terry.reedysetversions: - Python 3.2
2013年03月31日 19:26:42roger.serwysetfiles: + site_reversion.patch

messages: + msg185642
2013年03月31日 18:39:49pitrousetmessages: + msg185640
2013年03月31日 18:36:12roger.serwysetmessages: + msg185638
versions: + Python 3.2
2013年03月31日 18:26:59roger.serwysetfiles: + issue17585_2.7.patch
versions: + Python 2.7
nosy: + serhiy.storchaka

messages: + msg185637
2013年03月31日 18:15:15pitrousetmessages: + msg185636
2013年03月31日 18:14:58pitrousetnosy: + pitrou

messages: + msg185635
versions: - Python 2.7, Python 3.2
2013年03月31日 17:41:19roger.serwysetfiles: + issue17585.patch

messages: + msg185633
2013年03月31日 15:39:42terry.reedysetmessages: + msg185628
2013年03月31日 08:27:49roger.serwysetmessages: + msg185621
2013年03月31日 08:07:42terry.reedysetmessages: + msg185620
2013年03月31日 07:45:00roger.serwycreate

AltStyle によって変換されたページ (->オリジナル) /