classification
Title: Ensure in PyType_Ready() that base class of static type is static
Type: enhancement Stage: patch review
Components: Interpreter Core Versions: Python 3.5, Python 3.4, Python 2.7
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: serhiy.storchaka Nosy List: Arfrever, alex, doko, haypo, pitrou, python-dev, serhiy.storchaka, xiang.zhang
Priority: normal Keywords: needs review, patch

Created on 2014-07-26 12:36 by serhiy.storchaka, last changed 2016-06-27 02:21 by xiang.zhang.

Files
File name Uploaded Description Edit
issue22079.patch serhiy.storchaka, 2014-11-17 10:42 review
issue22079-tp_bases.patch serhiy.storchaka, 2016-06-26 06:28 review
Messages (20)
msg224049 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2014-07-26 12:36
It would be good if PyType_Ready() will check that base class of static type is static.
msg224102 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2014-07-27 04:13
By static, you mean not a heap type?
msg224103 - (view) Author: Alex Gaynor (alex) * (Python committer) Date: 2014-07-27 04:15
Yup.
msg224110 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2014-07-27 07:57
> By static, you mean not a heap type?

Yes. Sorry for bad wording.
msg231277 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2014-11-17 10:42
Here is a patch.
msg231579 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2014-11-23 21:32
Could anyone please make a review?
msg234852 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-01-27 20:59
Ping.
msg234876 - (view) Author: Roundup Robot (python-dev) Date: 2015-01-28 09:10
New changeset c087ac6fc171 by Serhiy Storchaka in branch '2.7':
Issue #22079: PyType_Ready() now checks that statically allocated type has
https://hg.python.org/cpython/rev/c087ac6fc171

New changeset 747855f29b9d by Serhiy Storchaka in branch '3.4':
Issue #22079: PyType_Ready() now checks that statically allocated type has
https://hg.python.org/cpython/rev/747855f29b9d

New changeset eb26255e11f1 by Serhiy Storchaka in branch 'default':
Issue #22079: PyType_Ready() now checks that statically allocated type has
https://hg.python.org/cpython/rev/eb26255e11f1
msg236807 - (view) Author: Matthias Klose (doko) * (Python committer) Date: 2015-02-27 17:59
reopening, this breaks some stuff in several places ...

https://bugs.launchpad.net/ubuntu/+source/terminator/+bug/1426294
msg236830 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-02-27 20:23
This restriction was added because bad thing could happen when statically allocated class inherits dynamically allocated class. I don't remember the number of the issue where such example was exposed, may be Alex remember it.

Likely pygobject creates classes in not safe manner. It would be more correct to create pygobject classes with PyType_FromSpec(), but current code worked for it for years. May be it is not so easy to change pygobject. So may be we should revert changes in 2.7 or convert the error to the deprecation warning.

Is there similar issue with 3.4? There may be difference due to the difference of the lifetime of types in 2.x and 3.x.
msg238881 - (view) Author: Roundup Robot (python-dev) Date: 2015-03-22 07:48
New changeset aa79a04e9bf5 by Serhiy Storchaka in branch '2.7':
Issue #22079: Py3k warning now is issued in  PyType_Ready() instead of
https://hg.python.org/cpython/rev/aa79a04e9bf5

New changeset 57a457ea84e4 by Serhiy Storchaka in branch '3.4':
Issue #22079: Deprecation warning now is issued in  PyType_Ready() instead of
https://hg.python.org/cpython/rev/57a457ea84e4
msg269188 - (view) Author: Xiang Zhang (xiang.zhang) * (Python committer) Date: 2016-06-24 15:33
I think we can start from index 1 instead of 0.
msg269195 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-06-24 17:15
Can you please explain?
msg269223 - (view) Author: Xiang Zhang (xiang.zhang) * (Python committer) Date: 2016-06-25 07:09
In the mro list I think the first is always the type itself which has already been checked to be a statically allocated type if we can touch the loop. Starting from 0 checks it twice and it's not a base either.

BTW, I don't receive an email from the tracker. :(
msg269247 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-06-25 19:21
__bases__ != mro()
msg269275 - (view) Author: Xiang Zhang (xiang.zhang) * (Python committer) Date: 2016-06-26 05:10
Of course, I know it. But doesn't the local variable `bases` refer to mro[1]?

[1] https://hg.python.org/cpython/file/tip/Objects/typeobject.c#l4900
msg269278 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-06-26 06:28
Good point! tp_bases should be iterated instead of tp_mro in this check.
msg269279 - (view) Author: Xiang Zhang (xiang.zhang) * (Python committer) Date: 2016-06-26 06:40
I thought that too at first. But if we use __bases__, can we still guarantee that 'all bases of statically allocated type should be statically allocated'? How about its grand bases? Is it possible that a statically allocated type inherit a dynamic type but does not go through PyType_Ready and then be inherited?
msg269291 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-06-26 15:02
This is interesting question. It looks that base classes (as well as mro classes) may not go through PyType_Ready. There is no even a check that mro() returns only ancestor classes and contains the initial class itself.
msg269339 - (view) Author: Xiang Zhang (xiang.zhang) * (Python committer) Date: 2016-06-27 02:21
Oh yes. The mro list can be customized so we can not even guarantee the type appears at index 0 and object at index n-1. It seems we should still keep what it is now, iterating the whole mro list.

Sorry that my previous comments seem to be noise. But maybe we can add one comment to clarify that iterating the whole list is needed.
History
Date User Action Args
2016-06-27 02:21:28xiang.zhangsetmessages: + msg269339
2016-06-26 15:02:27serhiy.storchakasetmessages: + msg269291
2016-06-26 06:40:23xiang.zhangsetmessages: + msg269279
2016-06-26 06:28:03serhiy.storchakasetstatus: closed -> open
files: + issue22079-tp_bases.patch
messages: + msg269278

resolution: fixed ->
stage: resolved -> patch review
2016-06-26 05:10:57xiang.zhangsetmessages: + msg269275
2016-06-25 19:21:09serhiy.storchakasetmessages: + msg269247
2016-06-25 07:09:13xiang.zhangsetmessages: + msg269223
2016-06-24 17:15:23serhiy.storchakasetmessages: + msg269195
2016-06-24 15:33:17xiang.zhangsetnosy: + xiang.zhang
messages: + msg269188
2015-03-30 07:05:09serhiy.storchakasetstatus: open -> closed
resolution: fixed
2015-03-22 13:02:52Arfreversetnosy: + Arfrever
2015-03-22 07:48:16python-devsetmessages: + msg238881
2015-02-27 20:23:51serhiy.storchakasetmessages: + msg236830
2015-02-27 17:59:08dokosetstatus: closed -> open

nosy: + doko
messages: + msg236807

resolution: fixed -> (no value)
2015-02-05 12:32:57serhiy.storchakasetstatus: open -> closed
assignee: serhiy.storchaka
resolution: fixed
stage: patch review -> resolved
2015-01-28 09:10:31python-devsetnosy: + python-dev
messages: + msg234876
2015-01-27 20:59:00serhiy.storchakasetmessages: + msg234852
2014-11-23 21:32:37serhiy.storchakasetmessages: + msg231579
2014-11-17 10:42:17serhiy.storchakasetkeywords: + needs review, patch
files: + issue22079.patch
messages: + msg231277

stage: needs patch -> patch review
2014-07-27 07:57:13serhiy.storchakasetmessages: + msg224110
2014-07-27 04:15:46alexsetmessages: + msg224103
2014-07-27 04:13:22pitrousetnosy: + pitrou
messages: + msg224102
2014-07-26 12:36:39serhiy.storchakacreate