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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
PEP 3121, 384 Refactoring applied to tkinter module #59926
Comments
Changes proposed in PEP-3121 and PEP-384 have now been applied to the tkinter module! >>> tkinter._test()
2012-08-18 12:46:24.775 python.exe[17410:707] speedup (null)
2012-08-18 12:46:24.776 python.exe[17410:707] ERROR: Unable to find method [_NSFullScreenTransition _startFullScreenTransitionForCGWindow:targetFrame:duration:].
2012-08-18 12:46:24.776 python.exe[17410:707] ERROR: Unable to find method [_NSFullScreenTransition _startEnterFullScreenTransitionForCGWindow:targetFrame:duration:].
2012-08-18 12:46:24.778 python.exe[17410:707] ERROR: Unable to find method [_NSFullScreenTransition startExitFullScreenTransitionForCGWindow:targetFrame:duration:].
[87221 refs]
>>> I have no Idea how and where these are triggered. |
See http://www.python.org/download/mac/tcltk/ It may be that using a different build of Tcl/Tk solves that problem. |
What version of 10.8 are you seeing those messages with? And do you see them without the patch applied? |
Update patch conforming to current _tkinter code. |
I would to have all module state inside _tkinterstate structure. |
Before I submitted this patch, I used to have these variables inside the modulestate, which caused severe problems. I do not know the exact reason, but my guess is that these variables have to be globally available for every thread (tcl variables are used for thread synchronization arent they?). As the modulestate may change depending on the thread, one can no longer guarantee that every thread within the process is operating on the same variable. This might not be nessecary, however as I mentioned, the naive approach of putting the variables inside the modulestate did not work out for me. |
I'm trying to make patch following myself recommendations. |
Submit patch which store own state into module state. |
Update patch to support TKINTER_PROTECT_LOADTK option. |
Upload version which check if _tkinter module was destroyed (for example, if it was call from daemon thread runing Tk app when main thread is exiting with interpreter finalization). |
The patch for current default branch. |
The patch is huge, then I like to apply it in two steps: first implement PEP-384 than 3121. |
New changeset bf9d118779f5 by Andrew Svetlov in branch 'default': |
Andrew, I have questions about the following part of commit bf9d118779f5: First, the "3" refers to the position of Py_tp_getattro in the array, which is a fragile thing IMO. Then, this hack was not necessary before; why is it needed now? |
Amaury, I completely agree with your objection. I have no idea how we can catch/reproduce the problem, maybe Martin von Loewis can help as author of xxlimited.c code? As I know almost nobody make PEP-384 compliant modules for now and maybe there are couple of dark corners at least in the docs. See also bpo-15650 for another question related to adaptation to PEP-3121. |
Amaury, you are right: |
New changeset 4eb6e07e8171 by Andrew Svetlov in branch 'default': |
Is any more work needed on this issue or can it be closed? |
The first two patches (_tkinter_pep3121-384_v0.patch and _tkinter_pep3121-384_v1.patch) seemed to have the refcounts right, but Andrew's further patches lost the required Py_INCREF in the various new() functions (and the corresponding Py_DECREF in deallocs), which is the cause for the crash in bpo-15667. |
New changeset 4d0c938870bc by Antoine Pitrou in branch 'default': |
_tkinter classes are now heap types and they no longer have the __module__ attribute. >>> import _tkinter
>>> _tkinter.TkappType.__module__
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
AttributeError: __module__ See bpo-20204. |
This change causes a crash when create instances of _tkinter classes (bpo-23815). If this will not be fixed I suggest to revert the patch. |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: