gh-84632: IDLE: fix clipboard being cleared upon exit#26163
Conversation
|
Just a couple of quick notes:
|
|
I'm suggesting this as an immediate fix for the reported issue as it relates to IDLE. Making this more generally available would require figuring out how it best fits into tkinter's set of abstractions, documenting it, testing it thoroughly, etc. I'm all for it, but IMO we should open a separate issue for that and not have it further delay this fix for IDLE. |
|
I'm looking into the test failure. Surprisingly, it is consistent and I can reproduce it locally on Windows 10. |
terryjreedy
left a comment
There was a problem hiding this comment.
- name
- blame
Docstring looks good. I presume C code okay since patch works for me, but better if Serhiy checks.
|
When you're done making the requested changes, leave the comment: |
It appears that merging in the main branch fixed the failing tests. 🤷 |
|
I have made the requested changes; please review again. |
|
Thanks for making the requested changes! @terryjreedy: please review the changes made to this pull request. |
|
I rebuilt branch and |
Co-authored-by: Terry Jan Reedy <tjreedy@udel.edu>
| root.mainloop() | ||
| root.destroy() | ||
| capture_warnings(False) | ||
| import _tkinter |
There was a problem hiding this comment.
What if add del root and two gc.collect()? Would it crash?
There was a problem hiding this comment.
Why would adding those cause a crash? Sorry, I don't understand the reasoning here, would you care to expand on this?
There was a problem hiding this comment.
This shouldn't cause a crash, but the check Serhiy suggested is to ensure that the interpreter isn't required for finalisation (i.e. if the root gets garbage collected, things are not going to go horribly wrong - at least that's how I understood it).
There was a problem hiding this comment.
AFAICT nothing else in our codebase calls Tcl_Finalize, Tcl_FinalizeThread, or anything else that triggers Tcl's finalization.
In addition, calling these multiple times shouldn't be an issue according to the Tcl docs:
Tcl_Finalizecan be safely called more than once.
|
Note: This does not work on more up-to-date Linux (e.g. Arch, Debian testing). It's a Tk issue (idk why). |
Signed-off-by: Tal Einat <532281+taleinat@users.noreply.github.com>
Could you elaborate please? What exactly doesn't work, and in what way? |
|
On the two distros named (I haven't tested on e.g. Fedora), Tk clipboard content is cleared on exit regardless of the new call. I proved it was a Tk issue because the following also results in an empty clipboard: |
Oh, then that's different from what we've been trying to fix up until now, since on all platforms tested so far the above worked as expected; it only didn't work via tkinter. That should be reported as a Tcl/Tk bug, and shouldn't affect this issue and PR. |
|
This PR is stale because it has been open for 30 days with no activity. |
|
@serhiy-storchaka are we good to merge this? |
|
I just had Python segfault while running IDLE with this change. Let's hold off on merging for now. |
|
This missed the boat for inclusion in Python 3.9 which accepts security fixes only as of today. |
The way the clipboard works on X11, is that it only stores a reference to the data when copying, and only retrieves the actual data when a paste is requested. Thus, the source application must provide the data at paste time, otherwise the clipboard will be empty. The clipboard content will only persist on X11 if there's a clipboard manager running, that constantly "harvests" clipboard selections. |
I smoke-tested ( |
|
This PR is stale because it has been open for 30 days with no activity. |
This uses
Tcl_FinalizeThread()rather thanTcl_Finalize(), because the latter caused memory segmentation errors on Ubuntu 20.04.Tested locally on Windows 10 and Ubuntu 20.04.