classification
Title: IDLE Crashes on File Open Dialog when code window closed before other file opened
Type: behavior Stage: resolved
Components: IDLE, Windows Versions: Python 3.2, Python 3.3, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: terry.reedy Nosy List: Patrick.Walters, amaury.forgeotdarc, belopolsky, python-dev, roger.serwy, terry.reedy, william.barr
Priority: normal Keywords: patch

Created on 2010-11-08 19:02 by william.barr, last changed 2013-02-05 03:49 by Patrick.Walters. This issue is now closed.

Files
File name Uploaded Description Edit
issue10365.patch william.barr, 2010-12-03 08:21 Issue 10365 patch; adds protection to editwin flist argument in the event of editwin closure during askopenfile call review
issue10365_rev2.patch roger.serwy, 2011-12-14 19:13 review
issue10365_rev3.patch roger.serwy, 2012-05-27 19:11 review
Messages (18)
msg120793 - (view) Author: William Barr (william.barr) Date: 2010-11-08 19:02
Steps for reproduction:
1.  Open IDLE (Python 3.1.2)
2.  Open a .py file
3.  With the code window (not the shell window) in focus, Ctrl + O to bring up the open file dialog.  Do not select a file or press open.  
4.  Close the code window.  
5.  Select a file and try to open it.
6.  The IDLE process will terminate.  

This test was performed on Windows 7 Professional 32-bit as well as Windows XP Professional 32-bit.  

Python 3.1.2 (r312:79149, Mar 21 2010, 00:41:52) [MSC v.1500 32 bit (Intel)] on win32
msg120794 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2010-11-08 19:12
Reproduced in a console window:
C:\>c:\python31\python.exe -m idlelib.idle
Exception in Tkinter callback
Traceback (most recent call last):
  File "c:\python31\lib\tkinter\__init__.py", line 1399, in __call__
    return self.func(*args)
  File "c:\python31\lib\idlelib\IOBinding.py", line 177, in open
    self.editwin.flist.open(filename)
AttributeError: 'NoneType' object has no attribute 'flist'
msg121054 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2010-11-12 18:50
That suggests that the last statement needs to be guarded somehow.
I re-versioned to 3.2 because 3.1.final will be out very soon.
I expect 3.2 and 2.7 should have same problem.
msg121073 - (view) Author: William Barr (william.barr) Date: 2010-11-12 20:43
Ok.  I'll see if I can get some protection around that then.  

I did test the issue with 2.7, and I didn't find it.  The window didn't open, but it didn't generate an exception that would kill the IDLE process.
msg123203 - (view) Author: William Barr (william.barr) Date: 2010-12-03 08:21
Ok, attached is a patch that should make IDLE silently ignore this happening; upon looking into this, it was a rather trivial fix.  The open function was waiting on the return input from the askopenfile call, during which the calling window was closed, setting the editwin parameter to None according to close.  The patch just adds another try/except to catch the AttributeError raised when the non-extant editwin's flist is referenced.  

I did come up with a method to actually make it continue with the opening process (just save a copy of the editwin's flist before the askopenfile call, during which the editwin gets closed), but that seemed a bit kludgey and possibly dangerous;, however it *seems* to work without issue.  I can upload that patch as well if anyone would care to review it in addition to the attached patch.
msg123207 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2010-12-03 08:49
The indentation of the patch 'looks' wrong. That appears to be because you used tabs instead of spaces (as in the lines removed and I presume elsewhere in the file -- and because FireFox interprets tabs as 8 spaces. Please redo with spaces. I will try to look at it during 3.2 beta phase if no one beats me to it.
msg127749 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2011-02-02 19:03
Unexpected exception is not a crash.  Changing the type to "behavior."
msg149467 - (view) Author: Roger Serwy (roger.serwy) * (Python committer) Date: 2011-12-14 19:13
William's explanation in msg123203 for the cause of the error and the solution for keeping a reference to "flist" is good. IDLE has only one instance of FileList while running anyways.

Attached is a patch that behaves like William's description.

The modification to PyShell.py is necessary for a corner-case. If the last IDLE window is closed with the Open File dialog still open, then FileList.py and WindowList.py both call .quit(). Without the modification, IDLE closes after selecting a file since root.mainloop() returns and root.destroy() gets executed. 

The alternative (and simpler) method is to have the Open File dialog be modal, but AFAIK it can't be set to modal.
msg161692 - (view) Author: Roundup Robot (python-dev) Date: 2012-05-27 00:47
New changeset a2877fbabf95 by Terry Jan Reedy in branch '3.2':
Issue #10365: File open dialog now works instead of crashing
http://hg.python.org/cpython/rev/a2877fbabf95

New changeset 21862628a013 by Terry Jan Reedy in branch 'default':
Merge with 3.2
http://hg.python.org/cpython/rev/21862628a013

New changeset 4334964993b9 by Terry Jan Reedy in branch '2.7':
Issue #10365: File open dialog now works instead of crashing
http://hg.python.org/cpython/rev/4334964993b9

New changeset 7dae83057e83 by Terry Jan Reedy in branch '3.2':
#10365 Trim trailing whitespace
http://hg.python.org/cpython/rev/7dae83057e83

New changeset 630baa300173 by Terry Jan Reedy in branch '2.7':
#10365 Trim trailing whitespace
http://hg.python.org/cpython/rev/630baa300173
msg161694 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2012-05-27 01:22
I verified problem in 3.3 and when opening dialog from shell.
Patch fixes problem, including open idle, open dialog, close shell, select file, and now file is opened. Thanks Roger. I went ahead and applied fix. (But note: your patch had a 'blank' line with several spaces. Not allowed. We both should remember to run whitespace-at-end-of-line stripper.)

I did not close yet because I think the comments need improvement (and perhaps made into doc string). Hence 'needs (second) patch'.

      # If the current window has no filename and hasn't been
      # modified, we replace its contents (no loss).  Otherwise
      # we open a new window.  But we won't replace the
      # shell window (which has an interp(reter) attribute), which
      # gets set to "not modified" at every new prompt.

I think that means to say "If the current window is a never-modified new, empty, untitled edit window, load the file there." If it had any contents to replace, I believe it would be either titled or modified. So that seem misleading.

The 'otherwise..open a new window' is not true since a file can be opened (for writing) only once, and if it is already open, focus is simply shifted to its current window.

So I think the behavior can be summarized as "If the file is already open in an edit window, shift focus to that window. Otherwise open a new window (or use the current window if it is new, unmodified, and untitled) and load the file."

      # Also, make sure the current window has not been closed,
      # since it can be closed during the Open File dialog.

This seems misplaced where it is since overall we are *not* making sure that the current window is still open, but are taking measures to make sure open works even if it has been closed. I think that the comment that might be added before this block is that flist is being saved in case the parent window gets closed while the dialog is open.

I am also curious if you understand
 # Code for use outside IDLE:
or might this be obsolete?
msg161716 - (view) Author: Roger Serwy (roger.serwy) * (Python committer) Date: 2012-05-27 19:11
(I need to write an extension to auto-apply strip-trailing-whitespace on save.)

Terry, I agree that the comment needs improvement. The self.editwin reference to the window needs to be available so that .get_saved() doesn't fail. In retrospect, the original patch obscured this point.

Attached is issue10365_rev3.patch, meant to be used against the latest code. It refactors the shell detection as part of the conditional as well as clarifies the comments.

The "Code for use outside IDLE" is part of a test at the end of the file which uses a duck-typed "EditorWindow" object. This object has flist=None which allows this extra code to run.
msg161739 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2012-05-27 22:09
The only substantive change I see is replacing the four lines setting 'interp' with a defaulted getattr call. You seem to have missed my point that 'Otherwise, open a new window' is false. It must be that "flist.open(filename)" first checks to see if filename is open and if so, uses the existing window rather than opening a new one. (I would check this if I knew what class flist is and where it is defined.) This must be what happens with "flist.open(filename, self.loadfile)" as it loads the file into the existing window, renames itself, and updates the recent files list. I will make the code replacement and do some comment changes.
msg161740 - (view) Author: Roger Serwy (roger.serwy) * (Python committer) Date: 2012-05-27 22:16
You're right. I missed your point about flist.open shifting focus to the already opened file. FileList.py contains the open method.
msg162187 - (view) Author: Roundup Robot (python-dev) Date: 2012-06-03 00:25
New changeset 5b267381eea0 by Terry Jan Reedy in branch '2.7':
Issue 10365: Add and replace comments; condense defaulted attribute access.
http://hg.python.org/cpython/rev/5b267381eea0

New changeset 4f3d4ce8ac9f by Terry Jan Reedy in branch '3.2':
Issue 10365: Add and replace comments; condense defaulted attribute access.
http://hg.python.org/cpython/rev/4f3d4ce8ac9f

New changeset d9b7399d9e45 by Terry Jan Reedy in branch 'default':
Merge with 3.2 #10365
http://hg.python.org/cpython/rev/d9b7399d9e45
msg181310 - (view) Author: Patrick (Patrick.Walters) Date: 2013-02-04 06:37
Forgive me if I'm not following the correct process. But I believe I have seen this issue again in 3.3. Not sure I captured exactly what is needed from the command line. The file being openeed is from the Python Hands On Class Examples http://anh.cs.luc.edu/python/hands-on/

c:\python33\python.exe -m idlelib.idle
Exception in Tkinter callback
Traceback (most recent call last):
  File "c:\python33\lib\tkinter\__init__.py", line 1442, in __call__
    return self.func(*args)
  File "c:\python33\lib\idlelib\MultiCall.py", line 174, in handler
    doafterhandler.pop()()
  File "c:\python33\lib\idlelib\MultiCall.py", line 221, in <lambda>
    doit = lambda: self.bindedfuncs[triplet[2]][triplet[0]].remove(func)
ValueError: list.remove(x): x not in list
Traceback (most recent call last):
  File "c:\python33\lib\runpy.py", line 160, in _run_module_as_main
    "__main__", fname, loader, pkg_name)
  File "c:\python33\lib\runpy.py", line 73, in _run_code
    exec(code, run_globals)
  File "c:\python33\lib\idlelib\idle.py", line 11, in <module>
    idlelib.PyShell.main()
  File "c:\python33\lib\idlelib\PyShell.py", line 1477, in main
    root.mainloop()
  File "c:\python33\lib\tkinter\__init__.py", line 1038, in mainloop
    self.tk.mainloop(n)
msg181399 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2013-02-05 00:17
It seems like a logic error to try to remove something that is not there. But it is not obvious from the traceback that your problem has anything to do with *opening* a file. Unbinding should only happen when *closing* a file. So I suspect this is a different issue.

Please
1. copy and paste the 'sign-in' line that looks like this:
"Python 3.3.0 (v3.3.0:bd8afb90ebf2, Sep 29 2012, 10:57:17) [MSC v.1600 64 bit (AMD64)] on win32"
2. add the OS version (like 'Win 7')
3. say *exactly* what you do and what happens when. In particular, do you get the traceback when you close a file? Or only when you actually press the [Open] button.

Try different python files to make sure it is not a problem with the specific file. Does your IDLE otherwise seem to run normally?
msg181406 - (view) Author: Roger Serwy (roger.serwy) * (Python committer) Date: 2013-02-05 03:04
Patrick, see Issue8900. It described your problem.
msg181413 - (view) Author: Patrick (Patrick.Walters) Date: 2013-02-05 03:49
Thanks for the pointer to the other issue. It looks spot on.
History
Date User Action Args
2013-02-05 03:49:16Patrick.Walterssetmessages: + msg181413
2013-02-05 03:04:05roger.serwysetmessages: + msg181406
2013-02-05 00:17:02terry.reedysetmessages: + msg181399
2013-02-04 06:37:01Patrick.Walterssetnosy: + Patrick.Walters
messages: + msg181310
2012-06-03 00:26:17terry.reedysetstatus: open -> closed
resolution: fixed
stage: needs patch -> resolved
2012-06-03 00:25:09python-devsetmessages: + msg162187
2012-05-28 03:15:52terry.reedysetassignee: terry.reedy
2012-05-27 22:16:10roger.serwysetmessages: + msg161740
2012-05-27 22:09:16terry.reedysetmessages: + msg161739
2012-05-27 19:11:07roger.serwysetfiles: + issue10365_rev3.patch

messages: + msg161716
2012-05-27 01:23:13terry.reedysetversions: + Python 2.7, Python 3.3
2012-05-27 01:22:47terry.reedysetmessages: + msg161694
stage: needs patch
2012-05-27 00:47:16python-devsetnosy: + python-dev
messages: + msg161692
2011-12-14 19:13:22roger.serwysetfiles: + issue10365_rev2.patch
nosy: + roger.serwy
messages: + msg149467

2011-02-02 19:03:32belopolskysettype: crash -> behavior

messages: + msg127749
nosy: + belopolsky
2010-12-03 08:49:21terry.reedysetmessages: + msg123207
2010-12-03 08:21:37william.barrsetfiles: + issue10365.patch
keywords: + patch
messages: + msg123203
2010-11-12 20:43:25william.barrsetmessages: + msg121073
2010-11-12 18:50:13terry.reedysetnosy: + terry.reedy

messages: + msg121054
versions: + Python 3.2, - Python 3.1
2010-11-08 19:12:56amaury.forgeotdarcsetnosy: + amaury.forgeotdarc
messages: + msg120794
2010-11-08 19:02:57william.barrcreate