Skip to content

gh-153480: Stop IDLE crashing when a file open in the editor is deleted - #153481

Open
tonghuaroot wants to merge 5 commits into
python:mainfrom
tonghuaroot:idlelib-editor-last-mtime-oserror
Open

tonghuaroot wants to merge 5 commits into
python:mainfrom
tonghuaroot:idlelib-editor-last-mtime-oserror

Conversation

@tonghuaroot

@tonghuaroot tonghuaroot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

EditorWindow.focus_in_event runs on every <FocusIn> event and calls last_mtime, which called os.path.getmtime(self.io.filename) with no error handling. When the open file had been deleted or renamed by another program, refocusing the editor window let a FileNotFoundError escape the Tk event handler and broke the modified-on-disk reload check.

last_mtime now guards getmtime with try/except OSError and returns the last known mtime, so focus_in_event sees no change and does not prompt to reload a file that is gone. The regression test drives last_mtime as an unbound method against a duck-typed stub (no GUI) for both the deleted-file and no-filename cases.

… deleted

EditorWindow.focus_in_event calls last_mtime on every <FocusIn> event,
which passed the open file path to os.path.getmtime with no guard. When
another program deleted or renamed the file, refocusing the IDLE window
let a FileNotFoundError escape the Tk event handler. last_mtime now
returns the last known mtime when the path is inaccessible, so refocusing
no longer raises and does not prompt to reload a file that is gone.

@serhiy-storchaka serhiy-storchaka left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR — the FocusIn traceback is real, but the fallback return self.mtime runs too early in one case.

EditorWindow.__init__ calls io.set_filename(filename) for a filename that does not exist yet (python -m idlelib new.py), which goes set_saved(True)last_mtime() before self.mtime is assigned at line 315. On main that already aborts IDLE at startup with FileNotFoundError (a regression from gh-94523, shipped in 3.13.15, 3.14.7 and 3.15.0b1); with this patch it aborts with AttributeError: 'EditorWindow' object has no attribute 'mtime' instead.

last_mtime() should return None when there is no file or it cannot be stat'ed (not 0, which is a valid mtime on filesystems that do not keep timestamps). A new file then opens with mtime = None, and a file that is created by another program after opening still triggers the existing reload prompt.

When the file was there and is now gone (self.mtime is not None and mtime is None), "modified by another program, reload?" is the wrong question — reloading can only fail. Ask instead what to do with the buffer: Close, Save As or Ignore, with Save As as the default. tkinter.simpledialog.SimpleDialog(..., buttons=('Close', 'Save As', 'Ignore'), default=1, cancel=2) gives such a box; .go() returns the button index. Close → self.close(). Save As → self.io.save_as(event). Ignore → set self.mtime = None and self.set_saved(False), so the window is marked as unsaved but the question is not repeated on the next FocusIn.

This is an approximate plan; details can be changed later.

Please add a test for the not-yet-existing filename (a SimpleNamespace stub without mtime is enough to catch the AttributeError), expect None in test_deleted_file_does_not_raise, and tests for the deleted-file branch with the dialog mocked.

Two smaller things: the NEWS file belongs in Misc/NEWS.d/next/IDLE/, not Library/, and open(p, 'w').close() in the test can be Path(p).touch() or a with.

@bedevere-app

bedevere-app Bot commented Sep 15, 2026

Copy link
Copy Markdown

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

last_mtime() now returns None for a missing or un-stat-able file, which
also fixes the startup path where it runs before self.mtime is set. The
vanished-file case now offers Close, Save As or Ignore instead of a
reload that could only fail. Tests updated; NEWS moved to IDLE/.
@tonghuaroot

Copy link
Copy Markdown
Contributor Author

Done: last_mtime() now returns None for a missing or un-stat-able file (this also fixes the startup path where it runs before self.mtime is set), and the vanished-file case shows the Close/Save As/Ignore dialog. Tests and the NEWS (moved to IDLE/) updated.

I have made the requested changes; please review again

@bedevere-app

bedevere-app Bot commented Sep 17, 2026

Copy link
Copy Markdown

Thanks for making the requested changes!

@serhiy-storchaka: please review the changes made to this pull request.

@serhiy-storchaka serhiy-storchaka left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this works for me now: starting with a not yet existing file, Ignore, reload after the file is recreated, Close. One remaining problem with a cancelled Save As, see the comment.

Comment thread Lib/idlelib/editor.py Outdated
Comment on lines +1156 to +1160
elif choice == 1:
self.io.save_as(event)
else:
self.mtime = None
self.set_saved(False)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If Save As is cancelled, self.mtime keeps the old value, and closing the dialog gives the focus back to the editor, so the dialog pops up again immediately — I got 11 dialogs in 3 seconds. Also, set_saved(False) is not needed: if the buffer was modified, it is already unsaved, and if it was not (e.g. a log file opened for viewing), there is nothing to lose by closing the window. A successful Save As sets mtime again via set_saved(True).

Suggested change
elif choice == 1:
self.io.save_as(event)
else:
self.mtime = None
self.set_saved(False)
else:
self.mtime = None
if choice == 1:
self.io.save_as(event)

Please adjust the tests accordingly.

Comment on lines +1 to +4
IDLE no longer fails with a traceback when a file open in the editor is
deleted or renamed by another program; refocusing the window now offers to
close it, save the buffer as a new file, or keep editing. Opening a
not-yet-created file no longer prevents IDLE from starting.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
IDLE no longer fails with a traceback when a file open in the editor is
deleted or renamed by another program; refocusing the window now offers to
close it, save the buffer as a new file, or keep editing. Opening a
not-yet-created file no longer prevents IDLE from starting.
Fix IDLE failing to start when opening a file which does not exist yet.
Fix a traceback when a file open in the IDLE editor is deleted by another
program; IDLE now asks whether to close the window, save the file elsewhere,
or ignore it.

A cancelled Save As kept the old mtime and reprompted on every focus.
Forget the mtime for both Save As and Ignore; a successful Save As
restores it via set_saved(True). Reword the NEWS per review.
@tonghuaroot

Copy link
Copy Markdown
Contributor Author

Fixed: the dialog now forgets the mtime for both Save As and Ignore, so a cancelled Save As no longer reprompts (a successful one restores it via set_saved(True)). Dropped the set_saved(False), applied your NEWS wording, and adjusted the tests.

I have made the requested changes; please review again

@bedevere-app

bedevere-app Bot commented Sep 17, 2026

Copy link
Copy Markdown

Thanks for making the requested changes!

@serhiy-storchaka: please review the changes made to this pull request.

@serhiy-storchaka serhiy-storchaka left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Found one more case: modify the file, delete it, get the focus back, choose Close. See the comment.

Comment thread Lib/idlelib/editor.py
buttons=('Close', 'Save As', 'Ignore'),
default=1,
cancel=2)
choice = dialog.go()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With a modified buffer, Close asks whether to save, and the FocusIn event queued when this dialog closed is delivered inside that message box, so the dialog is shown a second time (and currently crashes on the message box's grab, a separate tkinter bug). Set self.mtime = None before showing the dialog, for all choices; a successful Save As sets it again.

@bedevere-app

bedevere-app Bot commented Sep 17, 2026

Copy link
Copy Markdown

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants