classification
Title: Mark static types newly converted to heap types as immutable: add Py_TPFLAGS_DISALLOW_INSTANTIATION type flag
Type: behavior Stage: patch review
Components: C API Versions: Python 3.10
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: christian.heimes, erlendaasland, pablogsal, serhiy.storchaka, shreyanavigyan, vstinner
Priority: Keywords: patch

Created on 2021-04-22 21:31 by pablogsal, last changed 2021-05-03 14:50 by pablogsal.

Files
File name Uploaded Description Edit
patch.diff erlendaasland, 2021-04-26 22:13
disablednew.diff erlendaasland, 2021-04-27 08:23
Pull Requests
URL Status Linked Edit
PR 25653 closed erlendaasland, 2021-04-27 07:20
PR 25721 merged vstinner, 2021-04-29 15:37
PR 25722 closed christian.heimes, 2021-04-29 15:58
PR 25733 closed vstinner, 2021-04-29 21:53
PR 25745 merged vstinner, 2021-04-30 11:02
PR 25748 merged erlendaasland, 2021-04-30 12:15
PR 25749 merged vstinner, 2021-04-30 12:31
PR 25750 merged erlendaasland, 2021-04-30 13:26
PR 25751 merged vstinner, 2021-04-30 15:46
PR 25753 merged vstinner, 2021-04-30 15:58
PR 25768 merged pablogsal, 2021-05-01 00:55
PR 25791 merged christian.heimes, 2021-05-01 19:52
PR 25854 merged pablogsal, 2021-05-03 14:34
Messages (62)
msg391634 - (view) Author: Pablo Galindo Salgado (pablogsal) * (Python committer) Date: 2021-04-22 21:31
I'm creating this issue and marking it as a deferred blocker to make sure we don't forget to check this (adding tests) before the release.

Check this message from Serhiy regarding heap typed concerns:

https://bugs.python.org/msg391598
msg391635 - (view) Author: Pablo Galindo Salgado (pablogsal) * (Python committer) Date: 2021-04-22 21:32
Serhiy's comment for reference:

Static type with tp_new = NULL does not have public constructor, but heap type inherits constructor from base class. As result, it allows to create instances without proper initialization, that can lead to crash. It was fixed for few standard heap types in issue23815, then reintroduced, then fixed again in issue42694. But it should be checked for every type without constructor.
msg391903 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2021-04-26 11:48
From Objects/typeobject.c:

        /* The condition below could use some explanation.
           It appears that tp_new is not inherited for static types
           whose base class is 'object'; this seems to be a precaution
           so that old extension types don't suddenly become
           callable (object.__new__ wouldn't insure the invariants
           that the extension type's own factory function ensures).
           Heap types, of course, are under our control, so they do
           inherit tp_new; static extension types that specify some
           other built-in type as the default also
           inherit object.__new__. */
        if (base != &PyBaseObject_Type ||
            (type->tp_flags & Py_TPFLAGS_HEAPTYPE)) {
            if (type->tp_new == NULL)
                type->tp_new = base->tp_new;
        }
msg391905 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2021-04-26 12:10
The explaining comment was added in f884b749103bad0724e2e4878cd5fe49a41bd7df.

The code itself is much older. It was originally added in 6d6c1a35e08b95a83dbe47dbd9e6474daff00354, then
weaked in c11e192d416e2970e6a06cf06d4cf788f322c6ea and strengthen in 29687cd2112c540a8a4d31cf3b191cf10db08412.

All types except static types which are direct descendants of object inherit tp_new from a base class.

We need to check which static types had tp_new = NULL before they were converted to heap types and set it to NULL manually after type creation.
msg391909 - (view) Author: Erlend E. Aasland (erlendaasland) * (Python triager) Date: 2021-04-26 12:33
> We need to check which static types had tp_new = NULL before they were converted to heap types

I did a quick git grep. Results pasted into the list over at https://discuss.python.org/t/list-of-built-in-types-converted-to-heap-types/8403.
msg391910 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2021-04-26 12:45
It is incomplete. For example arrayiterator did have tp_new = NULL (in other words, PyArrayIter_Type.tp_new was not set to non-NULL value).

In 3.9:

>>> import array
>>> type(iter(array.array('I')))()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: cannot create 'arrayiterator' instances

In 3.10:

>>> import array
>>> type(iter(array.array('I')))()
<array.arrayiterator object at 0x7f02f88db5c0>
msg391911 - (view) Author: Erlend E. Aasland (erlendaasland) * (Python triager) Date: 2021-04-26 12:48
> It is incomplete.

Yes, I know. I'm working on completing it manually. Some of the static structs were only partially initialised (most stop at tp_members). The rest of the struct (including tp_new) would then be initialised to zero.
msg391912 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2021-04-26 12:50
Thank you for your work Erlend.
msg391913 - (view) Author: Pablo Galindo Salgado (pablogsal) * (Python committer) Date: 2021-04-26 12:51
Serhiy, do you think this should be a release blocker for the beta?
msg391917 - (view) Author: Erlend E. Aasland (erlendaasland) * (Python triager) Date: 2021-04-26 13:06
> Thank you for your work Erlend.

Anytime! I've updated the list now. There may still be errors, but I think it's pretty accurate now.
msg391924 - (view) Author: Erlend E. Aasland (erlendaasland) * (Python triager) Date: 2021-04-26 13:24
Here's a plaintext version:

- 0b858cdd5d2021-01-04 Modules/cjkcodecs/multibytecodec.c
    - _multibytecodec.MultibyteCodec

- c8a87addb1 2021-01-04 Modules/pyexpat.c
    - pyexpat.xmlparser

- 75bf107c62 2021-01-02 Modules/arraymodule.c
    - array.arrayiterator

- dd39123970 2020-12-29 Modules/_functoolsmodule.c
    - functools.KeyWrapper
    - functools._lru_list_elem

- 6104013838 2020-12-18 Modules/_threadmodule.c
    - _thread.lock
    - _thread._localdummy

- a6109ef68d 2020-11-20 Modules/_sre.c
    - re.Pattern
    - re.Match
    - _sre.SRE_Scanner

- c8c4200b65 2020-10-26 Modules/unicodedata.c
    - unicodedata.UCD

- 256e54acdb 2020-10-01 Modules/_sqlite
    - sqlite3.Connection
    - sqlite3.Cursor

- 9031bd4fa4 2020-10-01 Modules/_sqlite
    - sqlite3.Row
    - sqlite3.Statement

- cb6db8b6ae 2020-09-27 Modules/_sqlite
    - sqlite3.Node
    - sqlite3.Cache

- 52a2df135c 2020-09-08 Modules/sha256module.c
    - _sha256.sha224
    - _sha256.sha256

- 2aabc3200b 2020-09-06 Modules/md5module.c Modules/sha1module.c Modules/sha512module.c
    - _md5.md5
    - _sha1.sha1
    - _sha512.sha384
    - _sha512.sha512

- e087f7cd43 2020-08-13 Modules/_winapi.c
    - winapi__overlapped.Overlapped

- c4862e333a 2020-06-17 Modules/_gdbmmodule.c
    - g_dbm.dbm

- bf69a8f99f 2020-06-16 Modules/_dbmmodule.c
    - _dbm.dbm

- 33f15a16d4 2019-11-05 Modules/posixmodule.c
    - posix.DirEntry
    - posix.ScandirIterator

- df69e75edc 2019-09-25 Modules/_hashopenssl.c
    - _hashlib.HASH

- f919054e53 2019-09-14 Modules/selectmodule.c
    - select.devpoll
    - select.kevent
    - select.poll

- 04f0bbfbed 2019-09-10 Modules/zlibmodule.c
    - zlib.Compress
    - zlib.Decompress

- 4f384af067 2012-10-14 Modules/_tkinter.c
    - _tkinter.Tcl_Obj
    - _tkinter.tktimertoken
    - _tkinter.tkapp

- bc07cb883e 2012-06-14 Modules/_curses_panel.c
    - _curses_panel.panel
msg391925 - (view) Author: Erlend E. Aasland (erlendaasland) * (Python triager) Date: 2021-04-26 13:30
Is there any way we can fix this in Objects/typeobject.c, or do we actually have to patch every single type manually?
msg391926 - (view) Author: Pablo Galindo Salgado (pablogsal) * (Python committer) Date: 2021-04-26 13:36
> Is there any way we can fix this in Objects/typeobject.c, or do we actually have to patch every single type manually?

That would be probably a bad idea since typeobject.c is supposed to be generic. We would be inverting the responsibility concerns to the base.
msg391928 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2021-04-26 13:38
I afraid that changing the corresponding code in Objects/typeobject.c will break existing user code. For now, it is safer to patch every single type manually. Later we can design a general solution.
msg391929 - (view) Author: Pablo Galindo Salgado (pablogsal) * (Python committer) Date: 2021-04-26 13:40
I am marking this as a release blocker, giving that this is probably going to involve a big change.
msg391931 - (view) Author: Erlend E. Aasland (erlendaasland) * (Python triager) Date: 2021-04-26 13:43
Is it worth it adding tests for this? I'm thinking a generic version of msg391910 (maybe Lib/test/test_uninitialised_heap_types.py) coupled with a dataset.
msg391934 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2021-04-26 13:47
I have had this workaround in _hashopenssl.c for a while. Can I get rid of the function with "{Py_tp_new, NULL}" or is there a more generic way to accomplish the same goal?

/* {Py_tp_new, NULL} doesn't block __new__ */
static PyObject *
_disabled_new(PyTypeObject *type, PyObject *args, PyObject *kwargs)
{
    PyErr_Format(PyExc_TypeError,
        "cannot create '%.100s' instances", _PyType_Name(type));
    return NULL;
}
msg391936 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2021-04-26 13:52
test_uninitialised_heap_types.py will need to import unrelited modules (some of which can be platform depending). And more modules will be added as more types be converted. I think it is better to add tests for different modules in corresponding module test files. They are pretty trivial, for example:

    def test_new_tcl_obj(self):
        self.assertRaises(TypeError, _tkinter.Tcl_Obj)

    @requires_curses_func('panel')
    def test_new_curses_panel(self):
        w = curses.newwin(10, 10)
        panel = curses.panel.new_panel(w)
        self.assertRaises(TypeError, type(panel))
msg391937 - (view) Author: Erlend E. Aasland (erlendaasland) * (Python triager) Date: 2021-04-26 13:56
You're right, that would become messy. Complementing existing test suites is a better approach.
msg391984 - (view) Author: Erlend E. Aasland (erlendaasland) * (Python triager) Date: 2021-04-26 20:30
Christian:
> Can I get rid of the function with "{Py_tp_new, NULL}" [...]

Unfortunately not. The workaround in 993e88cf08994f7c1e0f9f62fda4ed32634ee2ad does the trick though.
msg391992 - (view) Author: Erlend E. Aasland (erlendaasland) * (Python triager) Date: 2021-04-26 22:12
Pablo, I've made a patch for most of these (I've left out Christian's modules). I've only added a couple of tests for now. Do you want all in one PR?

$ git diff main --stat  
 Lib/test/test_array.py             | 4 ++++
 Lib/test/test_unicodedata.py       | 4 ++++
 Modules/_dbmmodule.c               | 1 +
 Modules/_functoolsmodule.c         | 2 ++
 Modules/_gdbmmodule.c              | 1 +
 Modules/_sre.c                     | 5 +++++
 Modules/_threadmodule.c            | 2 ++
 Modules/_winapi.c                  | 1 +
 Modules/arraymodule.c              | 1 +
 Modules/cjkcodecs/multibytecodec.c | 1 +
 Modules/posixmodule.c              | 2 ++
 Modules/pyexpat.c                  | 1 +
 Modules/unicodedata.c              | 1 +
 Modules/zlibmodule.c               | 2 ++
 14 files changed, 28 insertions(+)


I don't know why I included the sqlite3 and select types in msg391924; they are not affected by this issue. Somebody should double check that everything's covered.
msg391999 - (view) Author: Pablo Galindo Salgado (pablogsal) * (Python committer) Date: 2021-04-26 23:32
> Pablo, I've made a patch for most of these (I've left out Christian's modules). I've only added a couple of tests for now. Do you want all in one PR?

Yes, please. The second most important part here is also adding regression tests so this doesn't happen. These tests should also be added when new types are converted in the future.
msg392036 - (view) Author: Erlend E. Aasland (erlendaasland) * (Python triager) Date: 2021-04-27 07:26
> These tests should also be added when new types are converted in the future.

We should have a checklist for static to heap type conversion. I'm pretty sure I've seen something like that in one of Victor's blogs. A new section in the Dev Guide, maybe?
msg392040 - (view) Author: Erlend E. Aasland (erlendaasland) * (Python triager) Date: 2021-04-27 08:23
Alternative approach:

Add _PyType_DisabledNew to Include/cpython, and add {Py_tp_new, _PyType_DisabledNew} slot to affected types. Diff attached.

See GH discussion:
- https://github.com/python/cpython/pull/25653#issuecomment-827383246
- https://github.com/python/cpython/pull/25653#issuecomment-827390034
msg392042 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2021-04-27 08:30
LGTM
msg392044 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2021-04-27 08:36
Alternatively we can make PyType_FromSpec() setting tp_new to NULL if there is explicit {Py_tp_new, NULL} in slots.
msg392046 - (view) Author: Erlend E. Aasland (erlendaasland) * (Python triager) Date: 2021-04-27 08:41
> Alternatively we can make PyType_FromSpec() setting tp_new to NULL if there is explicit {Py_tp_new, NULL} in slots.

Yes. It's not as explicit (and self-documenting) as _PyType_DisabledNew, though. But it avoids expanding the C API.
msg392048 - (view) Author: Erlend E. Aasland (erlendaasland) * (Python triager) Date: 2021-04-27 08:58
For that to work, you'd have to add a flag in PyType_FromModuleAndSpec, set the flag if (slot->slot == Py_tp_new && slot->pfunc == NULL) in the slots for loop, and then reset tp_new _after_ PyType_Ready.
msg392057 - (view) Author: Erlend E. Aasland (erlendaasland) * (Python triager) Date: 2021-04-27 11:53
> The second most important part here is also adding regression tests so this doesn't happen.

I've started adding tests. I'm not sure if I need to wrap them with @cpython_only.

Some of the types are hidden deep inside the implementations, and I have a hard time fetching them. For example _functools._lru_list_elem and _thread._localdummy. Is it ok to leave those out?
msg392169 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2021-04-28 07:19
Is it ok to skip tests if it is too hard to write them.

Tests should be decorated with @cpython_only because other Python implementations can have working constructors for these types. It is an implementation detail that these types are implemented in C and instances are not properly initialized by default.
msg392170 - (view) Author: Erlend E. Aasland (erlendaasland) * (Python triager) Date: 2021-04-28 07:20
Thanks, Serhiy.
msg392297 - (view) Author: Erlend E. Aasland (erlendaasland) * (Python triager) Date: 2021-04-29 09:54
Serhiy, would you mind reviewing the PR? bpo-43974 will clean up the changes introduced to setup.py.
msg392414 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-04-30 10:46
New changeset 3bb09947ec4837de75532e21dd4bd25db0a1f1b7 by Victor Stinner in branch 'master':
bpo-43916: Add Py_TPFLAGS_DISALLOW_INSTANTIATION type flag (GH-25721)
https://github.com/python/cpython/commit/3bb09947ec4837de75532e21dd4bd25db0a1f1b7
msg392426 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-04-30 12:07
New changeset 0cad068ec174bbe33fb80460da56eb413f3b9359 by Victor Stinner in branch 'master':
bpo-43916: Remove _disabled_new() function (GH-25745)
https://github.com/python/cpython/commit/0cad068ec174bbe33fb80460da56eb413f3b9359
msg392427 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-04-30 12:56
New changeset 4908fae3d57f68694cf006e89fd7761f45003447 by Victor Stinner in branch 'master':
bpo-43916: PyStdPrinter_Type uses Py_TPFLAGS_DISALLOW_INSTANTIATION (GH-25749)
https://github.com/python/cpython/commit/4908fae3d57f68694cf006e89fd7761f45003447
msg392433 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-04-30 13:49
New changeset 387397f8a4244c983f4568c16a28842e3268fe5d by Erlend Egeberg Aasland in branch 'master':
bpo-43916: select.poll uses Py_TPFLAGS_DISALLOW_INSTANTIATION (GH-25750)
https://github.com/python/cpython/commit/387397f8a4244c983f4568c16a28842e3268fe5d
msg392434 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-04-30 14:05
New changeset 9746cda705decebc0ba572d95612796afd06dcd4 by Erlend Egeberg Aasland in branch 'master':
bpo-43916: Apply Py_TPFLAGS_DISALLOW_INSTANTIATION to selected types (GH-25748)
https://github.com/python/cpython/commit/9746cda705decebc0ba572d95612796afd06dcd4
msg392435 - (view) Author: Erlend E. Aasland (erlendaasland) * (Python triager) Date: 2021-04-30 14:10
Pablo & Serhiy, can we close this issue now?
msg392437 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-04-30 14:12
Currently, 33 types explicitly set the Py_TPFLAGS_DISALLOW_INSTANTIATION flag:

* PyStdPrinter_Type
* _curses.ncurses_version type
* _curses_panel.panel
* _dbm.dbm
* _gdbm.gdbm
* _hashlib.HASH
* _hashlib.HASHXOF
* _hashlib.HMAC
* _multibytecodec.MultibyteCodec
* _sre..SRE_Scanner
* _thread._localdummy
* _thread.lock
* _tkinter.Tcl_Obj
* _tkinter.tkapp
* _tkinter.tktimertoken
* _winapi.Overlapped
* _xxsubinterpretersmodule.ChannelID
* array.arrayiterator
* functools.KeyWrapper
* functools._lru_list_elem
* os.DirEntry
* os.ScandirIterator
* pyexpat.xmlparser
* re.Match
* re.Pattern
* select.poll
* sys.flags type
* sys.getwindowsversion() type
* sys.version_info type
* unicodedata.UCD
* zlib.Compress
* zlib.Decompress
* _xxsubinterpreters.ChannelID

---

Static types with tp_base=NULL (or tp_base=&PyBaseObject_Type) and tp_new=NULL get Py_TPFLAGS_DISALLOW_INSTANTIATION flag automatically. Example of 70 static types which gets this flag automatically:

* CArgObject
* EncodingMap
* Generic
* GenericAlias
* PyCapsule
* TaskStepMethWrapper
* Token.MISSING
* _RunningLoopHolder
* _asyncio.FutureIter
* _ctypes.CThunkObject
* _ctypes.StructParam_Type
* _ctypes._CData
* _elementtree._element_iterator
* _io._BytesIOBuffer
* _pickle.Pdata
* _pickle.PicklerMemoProxy
* _pickle.UnpicklerMemoProxy
* _xxsubinterpreters.ChannelID
* anext_awaitable
* async_generator
* async_generator_asend
* async_generator_athrow
* async_generator_wrapped_value
* builtin_function_or_method
* bytearray_iterator
* bytes_iterator
* callable_iterator
* classmethod_descriptor
* coroutine
* coroutine_wrapper
* decimal.ContextManager
* dict_itemiterator
* dict_items
* dict_keyiterator
* dict_keys
* dict_reverseitemiterator
* dict_reversekeyiterator
* dict_reversevalueiterator
* dict_valueiterator
* dict_values
* fieldnameiterator
* formatteriterator
* frame
* generator
* getset_descriptor
* hamt_array_node
* hamt_bitmap_node
* hamt_collision_node
* items
* iterator
* keys
* list_iterator
* list_reverseiterator
* longrange_iterator
* managedbuffer
* member_descriptor
* method-wrapper
* method_descriptor
* moduledef
* odict_iterator
* range_iterator
* set_iterator
* str_iterator
* symtable entry
* tuple_iterator
* types.Union
* values
* weakcallableproxy
* weakproxy
* wrapper_descriptor
msg392438 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-04-30 14:13
I remove the release blocker priority of this issue, since the main issue has been fixed.
msg392451 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-04-30 15:47
According to msg391924, more types should use Py_TPFLAGS_DISALLOW_INSTANTIATION:

* _md5.md5
* _sha1.sha1
* _sha256.sha224
* _sha256.sha256
* _sha512.sha384
* _sha512.sha512

* select.devpoll: I created PR 25751
* select.kevent: it implements a working tp_new method: "{Py_tp_new, select_kqueue},"

* sqlite3.Cache
* sqlite3.Connection
* sqlite3.Cursor
* sqlite3.Node
* sqlite3.Row
* sqlite3.Statement
msg392456 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-04-30 16:00
* _md5.md5
* _sha1.sha1
* _sha256.sha224
* _sha256.sha256
* _sha512.sha384
* _sha512.sha512

I created PR 25753.
msg392462 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-04-30 16:05
It seems like _sqlite3 heap types support regular instantiation and so that no change is needed.

* sqlite3.Cache

    {Py_tp_new, PyType_GenericNew},

* sqlite3.Connection

    {Py_tp_new, PyType_GenericNew},
    {Py_tp_init, pysqlite_connection_init},

* sqlite3.Cursor

    {Py_tp_new, PyType_GenericNew},
    {Py_tp_init, pysqlite_cursor_init},

* sqlite3.Node

    {Py_tp_new, PyType_GenericNew},

* sqlite3.Row

    {Py_tp_new, pysqlite_row_new},

* sqlite3.Statement

    {Py_tp_new, PyType_GenericNew},


$ ./python
Python 3.10.0a7+
>>> import _sqlite3
>>> conn=_sqlite3.Connection(":memory:")

>>> type(conn)()
TypeError: function missing required argument 'database' (pos 1)
>>> conn2 = type(conn)(":memory:")

>>> list(conn.execute("SELECT 1;"))
[(1,)]
>>> list(conn2.execute("SELECT 1;"))
[(1,)]
msg392464 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-04-30 16:20
New changeset 7dcf0f6db38b7ab6918608542d3a0c6da0a286af by Victor Stinner in branch 'master':
bpo-43916: select.devpoll uses Py_TPFLAGS_DISALLOW_INSTANTIATION (GH-25751)
https://github.com/python/cpython/commit/7dcf0f6db38b7ab6918608542d3a0c6da0a286af
msg392465 - (view) Author: Erlend E. Aasland (erlendaasland) * (Python triager) Date: 2021-04-30 16:20
Yes, the sqlite3 types are fine. I don't know why I included them in my earlier post.

I cleared the tp_new markings for the sqlite3 types on the Discourse post, since I can edit my posts over there, contrary to here on bpo :)
msg392472 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-04-30 16:40
New changeset 665c7746fca180266b121183c2d5136c547006e0 by Victor Stinner in branch 'master':
bpo-43916: _md5.md5 uses Py_TPFLAGS_DISALLOW_INSTANTIATION (GH-25753)
https://github.com/python/cpython/commit/665c7746fca180266b121183c2d5136c547006e0
msg392473 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-04-30 16:41
> bpo-43916: _md5.md5 uses Py_TPFLAGS_DISALLOW_INSTANTIATION (GH-25753)

With this change, all tests listed in this issue should be fixed.

But I didn't check the whole list of https://discuss.python.org/t/list-of-built-in-types-converted-to-heap-types/8403
msg392526 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-04-30 22:40
Status in Python 3.9.

5 types allow instantiation whereas they must not: BUG!!!

* _tkinter.Tcl_Obj
* _tkinter.tkapp
* _tkinter.tktimertoken
* zlib.Compress
* zlib.Decompress

Example of bug:

$ python3.9
Python 3.9.4
>>> import zlib
>>> obj=zlib.compressobj()
>>> obj2=type(obj)()
>>> obj2.copy()
Erreur de segmentation (core dumped)

---

The remaining types disallow instantiation with different ways.

Static type with tp_base=NULL and tp_new=NULL:

* _dbm.dbm
* _gdbm.gdbm
* _multibytecodec.MultibyteCodec
* _sre.SRE_Scanner
* _thread._localdummy
* _thread.lock
* _winapi.Overlapped
* _xxsubinterpretersmodule.ChannelID
* array.arrayiterator
* functools.KeyWrapper
* functools._lru_list_elem
* pyexpat.xmlparser
* re.Match
* re.Pattern
* unicodedata.UCD
* _xxsubinterpreters.ChannelID

Heap type setting tp_new to NULL after type creation:

* _curses_panel.panel

Static type setting tp_new to NULL after type creation:

* _curses.ncurses_version type
* sys.flags type
* sys.getwindowsversion() type
* sys.version_info type

Define a tp_new or tp_init function which always raise an exception:

* PyStdPrinter_Type
* _hashlib.HASH
* _hashlib.HASHXOF
* _hashlib.HMAC
* os.DirEntry
* os.ScandirIterator
* select.poll
msg392528 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-04-30 22:42
* _tkinter.Tcl_Obj
* _tkinter.tkapp
* _tkinter.tktimertoken

Oops, sorry: in Python 3.9, tp_new is set to NULL after these heap types are created. There is no bug for these 3 types.
msg392534 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-04-30 23:00
I checked " Types converted pre Python 3.9" and "Types converted in Python 3.9" of:
https://discuss.python.org/t/list-of-built-in-types-converted-to-heap-types/8403

(I didn't check "Types converted in Python 3.10" yet.)

These lists are not complete, for example select.kevent was not listed whereas it has a bug.


_struct.unpack_iterator can be modified to use Py_TPFLAGS_DISALLOW_INSTANTIATION: its tp_new method always raises an exception.


Bugs!

* select.kevent: BUG, use {Py_tp_new, PyType_GenericNew} slot which doesn't properly initialize kqueue_event_Object!

Need review:

* _ast.AST: use {Py_tp_new, PyType_GenericNew} and {Py_tp_init, ast_type_init} slots. What happens if tp_new is called without calling tp_init?

Implement tp_new (ok!):

* _abc._abc_data
* _json.Scanner
* _json.Encoder
* select.devpoll
* select.epoll
* select.kqueue
* _random.Random
* _struct.Struct 

Other cases (ok!):

* _ssl.SSLError inherits from OSError and reuses OSError structure (PyOSErrorObject): the inherited tp_new works as expected.
* select.poll: Py_TPFLAGS_DISALLOW_INSTANTIATION
msg392537 - (view) Author: Erlend E. Aasland (erlendaasland) * (Python triager) Date: 2021-04-30 23:10
> These lists are not complete, for example select.kevent was not listed whereas it has a bug.

If one of the admins could make that post into a wiki post, it would be open for editing, making it easier to fill the holes in the lists.
msg392545 - (view) Author: Pablo Galindo Salgado (pablogsal) * (Python committer) Date: 2021-04-30 23:39
> If one of the admins could make that post into a wiki post, it would be open for editing, making it easier to fill the holes in the lists.

Not sure what do you have in mind for the wiki, but the easiest is to open some GitHub issue somewhere or using https://discuss.python.org/
msg392546 - (view) Author: Pablo Galindo Salgado (pablogsal) * (Python committer) Date: 2021-04-30 23:43
> * _ast.AST: use {Py_tp_new, PyType_GenericNew} and {Py_tp_init, ast_type_init} slots. What happens if tp_new is called without calling tp_init?

I think this is fine since ast_type_init doesn't initialize any C fields and only mutates the object by using PyObject_SetAttr
msg392553 - (view) Author: Pablo Galindo Salgado (pablogsal) * (Python committer) Date: 2021-05-01 00:43
Something is wrong after commit 3bb09947ec4837de75532e21dd4bd25db0a1f1b7.

When building Python I am getting:

*** WARNING: renaming "_curses" since importing it failed: /home/pablogsal/github/python/master/build/lib.linux-x86_64-3.10-pydebug/_curses.cpython-310d-x86_64-linux-gnu.so: undefined symbol: _PyStructSequence_InitType
*** WARNING: renaming "_curses_panel" since importing it failed: No module named '_curses'
msg392554 - (view) Author: Pablo Galindo Salgado (pablogsal) * (Python committer) Date: 2021-05-01 00:48
The problem is that the curses module is using a function that requires to be built in the core:

#ifdef Py_BUILD_CORE
extern int _PyStructSequence_InitType(
    PyTypeObject *type,
    PyStructSequence_Desc *desc,
    unsigned long tp_flags);
#endif
msg392556 - (view) Author: Pablo Galindo Salgado (pablogsal) * (Python committer) Date: 2021-05-01 00:55
The build errors were in the CI, but because Python doesn't complain if it cannot build a module, then we never catched this. I have opened https://github.com/python/cpython/pull/25768 to fix it to unblock the release, since the curses module is broken currently.
msg392558 - (view) Author: Pablo Galindo Salgado (pablogsal) * (Python committer) Date: 2021-05-01 01:21
New changeset 558df9010915c8fe94f4d7f842e7c5aabbb06b14 by Pablo Galindo in branch 'master':
bpo-43916: Export the _PyStructSequence_InitType to fix build errors in the curses module (GH-25768)
https://github.com/python/cpython/commit/558df9010915c8fe94f4d7f842e7c5aabbb06b14
msg392570 - (view) Author: Erlend E. Aasland (erlendaasland) * (Python triager) Date: 2021-05-01 05:15
> Not sure what do you have in mind for the wiki

See https://meta.discourse.org/t/what-is-a-wiki-post/30801
msg392621 - (view) Author: Pablo Galindo Salgado (pablogsal) * (Python committer) Date: 2021-05-01 19:25
> See https://meta.discourse.org/t/what-is-a-wiki-post/30801

Gotcha, will try to do that as soon as possible
msg392632 - (view) Author: Pablo Galindo Salgado (pablogsal) * (Python committer) Date: 2021-05-01 20:33
I have transformed https://discuss.python.org/t/list-of-built-in-types-converted-to-heap-types/8403 into a wiki. Tell me if you need some alterations of something is missing.
msg392635 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2021-05-01 20:42
New changeset ddbef71a2c166a5d5dd168e26493973053a953d6 by Christian Heimes in branch 'master':
bpo-43916: Rewrite new hashlib tests, fix typo (GH-25791)
https://github.com/python/cpython/commit/ddbef71a2c166a5d5dd168e26493973053a953d6
msg392811 - (view) Author: Pablo Galindo Salgado (pablogsal) * (Python committer) Date: 2021-05-03 14:50
New changeset c2931d31f8ba7cf10044de276018c713ffc73592 by Pablo Galindo in branch 'master':
bpo-43916: Move the _PyStructSequence_InitType function to the internal API (GH-25854)
https://github.com/python/cpython/commit/c2931d31f8ba7cf10044de276018c713ffc73592
History
Date User Action Args
2021-05-03 14:50:30pablogsalsetmessages: + msg392811
2021-05-03 14:34:20pablogsalsetpull_requests: + pull_request24538
2021-05-01 20:42:42christian.heimessetmessages: + msg392635
2021-05-01 20:33:00pablogsalsetmessages: + msg392632
2021-05-01 19:52:38christian.heimessetpull_requests: + pull_request24480
2021-05-01 19:25:53pablogsalsetmessages: + msg392621
2021-05-01 05:15:46erlendaaslandsetmessages: + msg392570
2021-05-01 01:21:26pablogsalsetmessages: + msg392558
2021-05-01 00:55:26pablogsalsetmessages: + msg392556
2021-05-01 00:55:04pablogsalsetpull_requests: + pull_request24461
2021-05-01 00:48:58pablogsalsetmessages: + msg392554
2021-05-01 00:43:12pablogsalsetmessages: + msg392553
2021-04-30 23:43:42pablogsalsetmessages: + msg392546
2021-04-30 23:39:44pablogsalsetmessages: + msg392545
2021-04-30 23:10:58erlendaaslandsetmessages: + msg392537
2021-04-30 23:00:50vstinnersetmessages: + msg392534
2021-04-30 22:42:22vstinnersetmessages: + msg392528
2021-04-30 22:40:43vstinnersetmessages: + msg392526
2021-04-30 16:41:21vstinnersetmessages: + msg392473
2021-04-30 16:40:38vstinnersetmessages: + msg392472
2021-04-30 16:20:59erlendaaslandsetmessages: + msg392465
2021-04-30 16:20:01vstinnersetmessages: + msg392464
2021-04-30 16:05:08vstinnersetmessages: + msg392462
2021-04-30 16:00:10vstinnersetmessages: + msg392456
2021-04-30 15:58:07vstinnersetpull_requests: + pull_request24443
2021-04-30 15:47:23vstinnersetmessages: + msg392451
2021-04-30 15:46:32vstinnersetpull_requests: + pull_request24441
2021-04-30 14:13:31vstinnersetpriority: release blocker ->

messages: + msg392438
2021-04-30 14:12:51vstinnersetmessages: + msg392437
2021-04-30 14:10:23erlendaaslandsetmessages: + msg392435
2021-04-30 14:05:04vstinnersetmessages: + msg392434
2021-04-30 13:49:24vstinnersetmessages: + msg392433
2021-04-30 13:26:53erlendaaslandsetpull_requests: + pull_request24439
2021-04-30 12:56:30vstinnersetmessages: + msg392427
2021-04-30 12:31:04vstinnersetpull_requests: + pull_request24438
2021-04-30 12:15:21erlendaaslandsetpull_requests: + pull_request24437
2021-04-30 12:07:06vstinnersetmessages: + msg392426
2021-04-30 11:02:24vstinnersetpull_requests: + pull_request24434
2021-04-30 10:52:57vstinnersetcomponents: + C API
2021-04-30 10:52:47vstinnersettitle: Check that new heap types cannot be created uninitialised: add Py_TPFLAGS_DISALLOW_INSTANTIATION type flag -> Mark static types newly converted to heap types as immutable: add Py_TPFLAGS_DISALLOW_INSTANTIATION type flag
2021-04-30 10:52:14vstinnersettitle: Check that new heap types cannot be created uninitialised -> Check that new heap types cannot be created uninitialised: add Py_TPFLAGS_DISALLOW_INSTANTIATION type flag
2021-04-30 10:46:19vstinnersetmessages: + msg392414
2021-04-29 21:53:30vstinnersetpull_requests: + pull_request24424
2021-04-29 15:58:41christian.heimessetpull_requests: + pull_request24413
2021-04-29 15:37:04vstinnersetpull_requests: + pull_request24412
2021-04-29 09:54:26erlendaaslandsetmessages: + msg392297
2021-04-28 07:20:36erlendaaslandsetmessages: + msg392170
2021-04-28 07:19:21serhiy.storchakasetmessages: + msg392169
2021-04-27 11:53:09erlendaaslandsetmessages: + msg392057
2021-04-27 08:58:32erlendaaslandsetmessages: + msg392048
2021-04-27 08:41:25erlendaaslandsetmessages: + msg392046
2021-04-27 08:36:52serhiy.storchakasetmessages: + msg392044
2021-04-27 08:30:34christian.heimessetmessages: + msg392042
2021-04-27 08:23:03erlendaaslandsetfiles: + disablednew.diff

messages: + msg392040
2021-04-27 07:26:17erlendaaslandsetmessages: + msg392036
2021-04-27 07:20:32erlendaaslandsetstage: patch review
pull_requests: + pull_request24344
2021-04-26 23:32:56pablogsalsetmessages: + msg391999
2021-04-26 22:13:03erlendaaslandsetfiles: + patch.diff
keywords: + patch
2021-04-26 22:12:19erlendaaslandsetmessages: + msg391992
2021-04-26 20:30:19erlendaaslandsetmessages: + msg391984
2021-04-26 13:56:04erlendaaslandsetmessages: + msg391937
2021-04-26 13:52:27serhiy.storchakasetmessages: + msg391936
2021-04-26 13:47:31christian.heimessetnosy: + christian.heimes
messages: + msg391934
2021-04-26 13:43:59erlendaaslandsetmessages: + msg391931
2021-04-26 13:40:10pablogsalsetpriority: deferred blocker -> release blocker

messages: + msg391929
2021-04-26 13:38:40serhiy.storchakasetmessages: + msg391928
2021-04-26 13:36:35pablogsalsetmessages: + msg391926
2021-04-26 13:30:13erlendaaslandsetmessages: + msg391925
2021-04-26 13:24:42erlendaaslandsetmessages: + msg391924
2021-04-26 13:06:14erlendaaslandsetmessages: + msg391917
2021-04-26 12:51:13pablogsalsetmessages: + msg391913
2021-04-26 12:50:39serhiy.storchakasetmessages: + msg391912
2021-04-26 12:48:39erlendaaslandsetmessages: + msg391911
2021-04-26 12:45:25serhiy.storchakasetmessages: + msg391910
2021-04-26 12:33:26erlendaaslandsetmessages: + msg391909
2021-04-26 12:10:44serhiy.storchakasetmessages: + msg391905
2021-04-26 11:54:22shreyanavigyansetnosy: + shreyanavigyan
2021-04-26 11:48:11serhiy.storchakasetmessages: + msg391903
2021-04-26 11:31:25erlendaaslandsetnosy: + erlendaasland
2021-04-22 21:32:34pablogsalsetmessages: + msg391635
2021-04-22 21:31:58pablogsalcreate