classification
Title: Segmentation fault when create _tkinter objects
Type: crash Stage: resolved
Components: Extension Modules, Tkinter Versions: Python 3.6, Python 3.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: serhiy.storchaka Nosy List: Robin.Schreiber, amaury.forgeotdarc, asvetlov, loewis, ned.deily, pitrou, python-dev, serhiy.storchaka, terry.reedy
Priority: normal Keywords: patch

Created on 2015-03-30 19:56 by serhiy.storchaka, last changed 2016-05-08 18:29 by serhiy.storchaka. This issue is now closed.

Files
File name Uploaded Description Edit
tp_new_workaround.patch serhiy.storchaka, 2016-05-07 21:23 review
Messages (11)
msg239636 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-03-30 19:56
In 2.7 and 3.3:

>>> import _tkinter
>>> _tkinter.Tcl_Obj()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: cannot create '_tkinter.Tcl_Obj' instances
>>> _tkinter.TkttType()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: cannot create 'tktimertoken' instances

In 3.4+:

>>> import _tkinter
>>> _tkinter.Tcl_Obj()
Segmentation fault (core dumped)

And the same for _tkinter.TkttType.

Looks as this is a result of issue15721.
msg240034 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2015-04-03 20:26
With 3.4.3, I get the equivalent on Windows (a 'python has stopped working' box).
msg240275 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2015-04-08 15:36
Probably a PyType_Ready() missing.
msg242678 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-05-06 14:56
PyType_Ready() is called inside PyType_FromSpec().
msg254681 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-11-15 08:28
Before issue15721 the tp_new was NULL, and this prevented from creating instances. After issue15721 the tp_new is inherited from object. An instance is created, but all internal pointers are NULLs. Following repr() dereferences NULL.
msg265094 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-05-07 21:23
There is special exception -- tp_new is not inherited for static types whose base class is 'object' (see a comment in inherit_special() in Objects/typeobject.c#4569 for explanation). After converting them to heap types they became inherit tp_new from object.

Proposed patch adds workarounds for types in _tkinter and curses.panel modules.
msg265096 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2016-05-07 22:42
The news part of the patch did not apply  -- best not to include such in posted patches.

After recompiliing 3.5 on Windows 10, the two tkinter test cases give the 3.3 TypeErrors instead of crashing.  test_tcl also runs.  curses does not run on Windows.

I do not know enough internals to really understand #15721 or why you call the fix a workaround, but I do understand 'exception' (OK) versus 'crash' (forbidden).  Unless you think restoring the previous situation (tp_new = NUll) can cause a worse problem, I would apply this soon, within a week, so not forgotten before the June update releases.  The tcl test addition is correct.  The fix can be improved later if you think of something better.

I initially wondered why _tkinter has classes that cannot be instantiated (from Python).  Then I discovered that this is normal, and that the exception is standard.

>>> type(iter([]))()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: cannot create 'list_iterator' instances

Is list_iterator.tp_new NULL?  Or does it raise instead of crash by other means?
msg265102 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-05-07 23:02
I called the fix a workaround because other solution may be to make PyType_FromSpec() not setting tp_new in such cases. I expect that this is common mistake when convert a static class to a heap class with PyType_FromSpec(). At least we should add warnings in C API documentation and PEP 384. Or change the behavior of PyType_FromSpec() if it is appropriate. But this is different issue. If PyType_FromSpec() will be changed, proposed fix can be reverted.
msg265106 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2016-05-08 00:12
I noticed that you reported the crash and asked for a fix or reversion on #15721 6 months ago, with no response.  Hence, lets not wait longer.

Looking again, I see that there were 3 rounds of patches in 2012/2013.  If you want to follow up, I suggest closing that issue and open a fresh issue with with alternatives and your recommendation.
msg265150 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-05-08 17:47
New changeset cd25508c62fc by Serhiy Storchaka in branch '3.5':
Issue #23815: Fixed crashes related to directly created instances of types in
https://hg.python.org/cpython/rev/cd25508c62fc

New changeset 1c6326e81c33 by Serhiy Storchaka in branch 'default':
Issue #23815: Fixed crashes related to directly created instances of types in
https://hg.python.org/cpython/rev/1c6326e81c33
msg265153 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-05-08 18:29
After looking closer I found that making PyType_FromSpec() not inheriting tp_new for static types whose base class is 'object' is not an option, because this breaks examples in the documentation, and likely break some third-party code. Opened new issue26979 for documenting the catch.

Thank you for your review Terry.
History
Date User Action Args
2016-05-08 18:29:33serhiy.storchakasetstatus: open -> closed
resolution: fixed
messages: + msg265153

stage: patch review -> resolved
2016-05-08 17:47:18python-devsetnosy: + python-dev
messages: + msg265150
2016-05-08 00:12:51terry.reedysetmessages: + msg265106
2016-05-07 23:02:44serhiy.storchakasetmessages: + msg265102
2016-05-07 22:42:11terry.reedysetmessages: + msg265096
2016-05-07 21:23:38serhiy.storchakasetfiles: + tp_new_workaround.patch
versions: - Python 3.4
messages: + msg265094

assignee: serhiy.storchaka
keywords: + patch
stage: patch review
2015-11-15 08:28:13serhiy.storchakasetmessages: + msg254681
versions: + Python 3.6
2015-05-06 14:56:16serhiy.storchakasetmessages: + msg242678
2015-04-08 15:36:52amaury.forgeotdarcsetmessages: + msg240275
2015-04-03 20:26:47terry.reedysetnosy: + terry.reedy
messages: + msg240034
2015-03-30 19:56:54serhiy.storchakacreate