Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

gh-84632: IDLE: fix clipboard being cleared upon exit#26163

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Open
taleinat wants to merge9 commits intopython:main
base:main
Choose a base branch
Loading
fromtaleinat:bpo-40452/IDLE-preserve-clipboard-upon-exit

Conversation

taleinat
Copy link
Contributor

@taleinattaleinat commentedMay 16, 2021
edited by terryjreedy
Loading

This usesTcl_FinalizeThread() rather thanTcl_Finalize(), because the latter caused memory segmentation errors on Ubuntu 20.04.

Tested locally on Windows 10 and Ubuntu 20.04.

@E-Paine
Copy link
Contributor

Just a couple of quick notes:

  1. Is it worth adding this to the regular tkinter module? I suspect it would be useful for applications other than IDLE (I would personally make great use of such a method). We would have to ensure it is properly documented so people know when / not to use it.

  2. I suspect we would need to add a flag to ensure the user doesn't / cannot try to create a new Tk instance after calling the Tcl_Finalize wrapper (i.e. an exception would be raised by_tkinter.create)

@taleinat
Copy link
ContributorAuthor

taleinat commentedMay 16, 2021
edited
Loading

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.

E-Paine reacted with thumbs up emoji

@taleinat
Copy link
ContributorAuthor

I'm looking into the test failure. Surprisingly, it is consistent and I can reproduce it locally on Windows 10.

Copy link
Member

@terryjreedyterryjreedy left a comment

Choose a reason for hiding this comment

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

  1. name
  2. blame

Docstring looks good. I presume C code okay since patch works for me, but better if Serhiy checks.

@bedevere-bot
Copy link

When you're done making the requested changes, leave the comment:I have made the requested changes; please review again.

@taleinat
Copy link
ContributorAuthor

taleinat commentedMay 16, 2021
edited
Loading

I'm looking into the test failure. Surprisingly, it is consistent and I can reproduce it locally on Windows 10.

It appears that merging in the main branch fixed the failing tests. 🤷

@taleinat
Copy link
ContributorAuthor

I have made the requested changes; please review again.

@bedevere-bot
Copy link

Thanks for making the requested changes!

@terryjreedy: please review the changes made to this pull request.

@terryjreedy
Copy link
Member

I rebuilt branch andhelp(_tkinter._finalize_tcl) properly displays the new docstring. Once the blurb is fixed, I'm okay with you merging when you are.

Co-authored-by: Terry Jan Reedy <tjreedy@udel.edu>
@@ -1698,6 +1698,8 @@ def main():
root.mainloop()
root.destroy()
capture_warnings(False)
import _tkinter

Choose a reason for hiding this comment

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

What if adddel root and twogc.collect()? Would it crash?

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

Why would adding those cause a crash? Sorry, I don't understand the reasoning here, would you care to expand on this?

Copy link
Contributor

@E-PaineE-PaineMay 19, 2021
edited
Loading

Choose a reason for hiding this comment

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

Thisshouldn'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).

Copy link
ContributorAuthor

@taleinattaleinatMay 19, 2021
edited
Loading

Choose a reason for hiding this comment

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

AFAICT nothing else in our codebase callsTcl_Finalize,Tcl_FinalizeThread, or anything else that triggers Tcl's finalization.

In addition, calling these multiple times shouldn't be an issue according tothe Tcl docs:

Tcl_Finalize can be safely called more than once.

@E-Paine
Copy link
Contributor

E-Paine commentedMay 17, 2021
edited
Loading

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>
@taleinat
Copy link
ContributorAuthor

Note: This does not work on more up-to-date Linux (e.g. Arch, Debian testing). It's a Tk issue (idk why).

Could you elaborate please? What exactly doesn't work, and in what way?

@E-Paine
Copy link
Contributor

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:

>>> wish% clipboard clear% clipboard append {testing 123}% exit

@taleinat
Copy link
ContributorAuthor

taleinat commentedMay 19, 2021
edited
Loading

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:

>>> wish% clipboard clear% clipboard append {testing 123}% exit

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.

@taleinat
Copy link
ContributorAuthor

@serhiy-storchaka, with the latest change based on your review comment, do you think this is good to go?

@github-actions
Copy link

This PR is stale because it has been open for 30 days with no activity.

@github-actionsgithub-actionsbot added the staleStale PR or inactive for long period of time. labelJun 19, 2021
@E-Paine
Copy link
Contributor

@serhiy-storchaka are we good to merge this?

@taleinat
Copy link
ContributorAuthor

I just had Python segfault while running IDLE with this change. Let's hold off on merging for now.

@github-actionsgithub-actionsbot removed the staleStale PR or inactive for long period of time. labelJul 7, 2021
@ambvambv removed the needs backport to 3.9only security fixes labelMay 17, 2022
@ambv
Copy link
Contributor

This missed the boat for inclusion in Python 3.9 which accepts security fixes only as of today.

@terryjreedyterryjreedy changed the titlebpo-40452: IDLE: fix clipboard being cleared upon exitgh-84632: IDLE: fix clipboard being cleared upon exitSep 18, 2022
@rdbende
Copy link

Note: This does not work on more up-to-date Linux (e.g. Arch, Debian testing). It's a Tk issue (idk why).

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.

@hugovkhugovk removed the needs backport to 3.10only security fixes labelApr 8, 2023
@arhadthedev
Copy link
Member

I just had Python segfault while running IDLE with this change. Let's hold off on merging for now.

I smoke-tested (python -m idlelib-select-Ctrl+C-close) IDLE from the head of this PR and couldn't reproduce the crash.@taleinat could you recheck if the underlying reason was fixed during last two years, please?

@serhiy-storchakaserhiy-storchaka added needs backport to 3.12only security fixes needs backport to 3.13bugs and security fixes and removed needs backport to 3.11only security fixes labelsMay 9, 2024
@tomasr8tomasr8 removed the needs backport to 3.12only security fixes labelApr 10, 2025
@python-cla-bot
Copy link

python-cla-botbot commentedApr 18, 2025
edited
Loading

All commit authors signed the Contributor License Agreement.

CLA signed

@serhiy-storchakaserhiy-storchaka added the needs backport to 3.14bugs and security fixes labelMay 8, 2025
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@serhiy-storchakaserhiy-storchakaserhiy-storchaka left review comments

@E-PaineE-PaineE-Paine left review comments

@terryjreedyterryjreedyAwaiting requested review from terryjreedyterryjreedy is a code owner

Assignees
No one assigned
Labels
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

12 participants
@taleinat@E-Paine@bedevere-bot@terryjreedy@ambv@rdbende@arhadthedev@serhiy-storchaka@hugovk@tomasr8@the-knights-who-say-ni@ezio-melotti

[8]ページ先頭

©2009-2025 Movatter.jp