array.array should remain immutable: add Py_TPFLAGS_IMMUTABLETYPE flag

[gvanrossum]

BPO	43908
Nosy	@gvanrossum, @vstinner, @tiran, @markshannon, @serhiy-storchaka, @gvanrossum, @corona10, @pablogsal, @miss-islington, @shihai1991, @erlend-aasland, @shreyanavigyan
PRs	bpo-43908: Add Py_TPFLAGS_IMMUTABLETYPE flag #25520 bpo-43908: Make array.array type immutable #25696 bpo-43908: Make re types immutable #25697 bpo-43908: Document Static Types in the C API #25710 bpo-43973: Use Py_TPFLAGS_IMMUTABLETYPE to check for class assignments #25714 bpo-43908: check_set_special_type_attr() and type_set_annotations() now check for immutable flag #25743 bpo-43908: Mark ssl, hash, and hmac types as immutable (GH-25792) #25792 bpo-43908: Add What's New entry for Py_TPFLAGS_IMMUTABLETYPE flag #25816 [3.10] bpo-43908: Add What's New entry for Py_TPFLAGS_IMMUTABLETYPE flag (GH-25816) #26115 bpo-43908: Make heap types converted during 3.10 alpha immutable #26351 [3.10] bpo-43908: Make heap types converted during 3.10 alpha immutable (GH-26351) #26766 bpo-43908: Types with Py_TPFLAGS_IMMUTABLETYPE can now inherit the vectorcall protocol #27001
Files	patch.diff apply-to-all.diff list.txt apply-to-all.diff: modified tests_log.txt

^{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:

assignee = 'https://github.com/vstinner'
closed_at = None
created_at = <Date 2021-04-21.18:52:19.855>
labels = ['type-bug', 'library', '3.10', '3.11']
title = 'array.array should remain immutable: add Py_TPFLAGS_IMMUTABLETYPE flag'
updated_at = <Date 2022-01-26.17:39:44.786>
user = 'https://github.com/gvanrossum'

bugs.python.org fields:

activity = <Date 2022-01-26.17:39:44.786>
actor = 'vstinner'
assignee = 'vstinner'
closed = False
closed_date = None
closer = None
components = ['Library (Lib)']
creation = <Date 2021-04-21.18:52:19.855>
creator = 'gvanrossum'
dependencies = []
files = ['49970', '49980', '49987', '49990', '49994']
hgrepos = []
issue_num = 43908
keywords = ['patch']
message_count = 120.0
messages = ['391540', '391578', '391582', '391584', '391585', '391595', '391596', '391597', '391598', '391600', '391636', '391701', '391720', '391728', '391730', '391756', '391816', '391846', '391866', '391867', '391868', '391895', '391901', '391918', '391922', '391930', '391932', '391933', '391935', '391938', '391946', '391947', '391954', '391959', '391960', '391963', '391964', '391971', '391993', '392029', '392038', '392076', '392079', '392080', '392083', '392085', '392086', '392087', '392088', '392089', '392092', '392098', '392101', '392103', '392105', '392236', '392237', '392238', '392240', '392247', '392248', '392250', '392251', '392262', '392263', '392264', '392283', '392284', '392285', '392286', '392287', '392288', '392311', '392312', '392314', '392316', '392318', '392324', '392325', '392389', '392402', '392403', '392407', '392408', '392409', '392410', '392413', '392416', '392420', '392421', '392422', '392430', '392439', '392440', '392441', '392443', '392445', '392446', '392488', '392638', '392670', '392671', '392693', '393618', '394314', '394315', '394319', '395178', '395983', '395988', '396829', '396830', '397134', '397163', '397164', '397170', '397171', '397232', '411782', '411784']
nosy_count = 12.0
nosy_names = ['gvanrossum', 'vstinner', 'christian.heimes', 'Mark.Shannon', 'serhiy.storchaka', 'Guido.van.Rossum', 'corona10', 'pablogsal', 'miss-islington', 'shihai1991', 'erlendaasland', 'shreyanavigyan']
pr_nums = ['25520', '25696', '25697', '25710', '25714', '25743', '25792', '25816', '26115', '26351', '26766', '27001']
priority = None
resolution = None
stage = 'patch review'
status = 'open'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue43908'
versions = ['Python 3.10', 'Python 3.11']

[gvanrossum]

Hi Victor,

Sorry for making this a deferred blocker. I recall that we had a brief discussion somewhere about an accidental change to the array.array type -- this is now a heap type (Py_TPFLAGS_HEAPTYPE is set), and as a consequence it is no longer immutable.

In 3.9 this is an error:

>>> import array        
>>> array.array.foo = 1
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: can't set attributes of built-in/extension type 'array.array'
>>>

But in 3.10a7 it passes:

>>> array.array.foo = 1
>>> array.array.foo    
1
>>>

I would like this type (and other types that have been or will be converted to heap types) to remain immutable. How can we do that? I think we may need a new flag bit meaning "built-in type, immutable". This bit should not be inherited of course.

What do you think? (Feel free to close if this is a duplicate -- I couldn't find where we discussed this previously, sorry.)

[shihai1991]

I would like this type (and other types that have been or will be converted to heap types) to remain immutable.

Hi, Guido. I am interested in what have been broken in fact? or it's a defined behavior?

I think we may need a new flag bit meaning "built-in type, immutable". This bit should not be inherited of course.

Aggreed. Adding a new tp_flag is a lightweight solution.

[erlend-aasland]

FWIW, I was the one who converted arraymodule to use heap types in 75bf107 (GH-23124) in January. Sorry 'bout the accidental change of behaviour.

I think we may need a new flag bit meaning "built-in type, immutable". This bit should not be inherited of course.

+1.

See attached PoC patch. As far as I understand, type flags must be explicitly inherited in Objects/typeobject.c, no?

Should all previous static types be immutable, or are there exceptions?

[markshannon]

Adding an extra flag seems like the sensible thing to do for 3.10

Longer term, we should decouple immutability from whether something is a heap type.
I don't know why we would care that it is a heap type at all. Which bit of memory it happens to sit in seems irrelevant to anything but the GC.

Erland, could you turn your patch into PR?

[erlend-aasland]

Erland, could you turn your patch into PR?

Certainly, but despite the issue title, this issue mentions all types that have been converted to heap types. Should we apply the new immutable flag to _all_ heap types in the stdlib?

Guido:

I would like this type (and other types that have been or will be converted to heap types) to remain immutable.

[vstinner]

Guido:

Sorry for making this a deferred blocker. I recall that we had a brief discussion somewhere about an accidental change to the array.array type -- this is now a heap type (Py_TPFLAGS_HEAPTYPE is set), and as a consequence it is no longer immutable.

*Many* static types have been converted to heap types in Python 3.9 and Python 3.10. Is there a rule to decide which types should be mutable or not?

All types implemented in Python are mutable, unless the very few which use slots.

By the way, some cases can be inherited or not. Do we care about that?

Example:

$ python3
Python 3.9.2 (default, Feb 20 2021, 00:00:00) 
>>> def f(): pass
... 
>>> class MyFuncType(type(f)): pass
... 
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: type 'function' is not an acceptable base type

[vstinner]

Let me propose a flag name: Py_TPFLAGS_IMMUTABLE.

Py_TPFLAGS_IMMUTABLE implies that setattr(type, name, value) fails. A type has a dictionary, but it cannot be modified with type.__dict__[name] = value, since type.__dict__ returns a read-only mapping proxy.

Currently, the limitation of not being able to modify a built-in type is arbitrary, it's not a technical limitation. It is implemented in type_setattro() which is the default implementation of tp_setattro: PyType_Type.tp_setattro = type_setattro.

Not having Py_TPFLAGS_HEAPTYPE flag implies "immutable". I suggest to add a new Py_TPFLAGS_IMMUTABLE flag and modify PyType_Ready() to use the flag if Py_TPFLAGS_HEAPTYPE is not set:

if (!(flags & Py_TPFLAGS_HEAPTYPE)) {
   flags |= Py_TPFLAGS_IMMUTABLE;
}

You can try with this change:

diff --git a/Objects/typeobject.c b/Objects/typeobject.c
index 254d12cc97..4bd02d40c1 100644
--- a/Objects/typeobject.c
+++ b/Objects/typeobject.c
@@ -3875,13 +3875,6 @@ static int
 type_setattro(PyTypeObject *type, PyObject *name, PyObject *value)
 {
     int res;
-    if (!(type->tp_flags & Py_TPFLAGS_HEAPTYPE)) {
-        PyErr_Format(
-            PyExc_TypeError,
-            "can't set attributes of built-in/extension type '%s'",
-            type->tp_name);
-        return -1;
-    }
     if (PyUnicode_Check(name)) {
         if (PyUnicode_CheckExact(name)) {
             if (PyUnicode_READY(name) == -1)

It becomes possible to modify built-in types:

$ ./python
Python 3.10.0a7+ (heads/master-dirty:cdad2724e6, Apr 22 2021, 14:44:44) [GCC 10.2.1 20201125 (Red Hat 10.2.1-9)] on linux
>>> str.x=1
>>> str.x
1
>>> del str.x

>> del str.__repr__

>>> del str.__repr__
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AttributeError: __repr__

[serhiy-storchaka]

Maybe name it Py_TPFLAGS_IMMUTABLETYPE? Just IMMUTABLE can mean that instances of the type are immutable, but we want to make a type itself immutable.

[serhiy-storchaka]

Is there a full list of types converted to heap types? There may be other issues related to differences between heap and static types.

1. 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 bpo-23815, then reintroduced, then fixed again in bpo-42694. But it should be checked for every type without constructor.

2. Static types was not pickleable with protocol 0 and 1 by default. Heap types were pickleable by default, as result, you did not get error when pickle instances of newly converted types, but get an error when try to unpickle. It was fixed in general enough case in bpo-41052, but it is worth to recheck it for every converted type.

[vstinner]

Maybe name it Py_TPFLAGS_IMMUTABLETYPE?

"TPFLAGS" stands for "type flags". So IMO adding "TYPE" suffix is redundant. If tomorrow, we want to add a flag for immutable instance, we can add a new Py_TPFLAGS_IMMUTABLE_INSTANCE flag.

On the other side, TYPE suffix is consistent with other flags :-)

/* Set if the type object is dynamically allocated */
#define Py_TPFLAGS_HEAPTYPE (1UL << 9)

/* Set if the type allows subclassing */
#define Py_TPFLAGS_BASETYPE (1UL << 10)

[erlend-aasland]

Victor, I've updated PR 25520 so PyType_Ready always sets Py_TPFLAGS_IMMUTABLE for static types.

[gvanrossum]

Is there going to be a separate PR to actually *set* the IMMUTABLE flag on all built-in type objects? There are many of those.

[erlend-aasland]

Guido, I’ll prepare a separate PR for all converted built-in types, and you can decide what to do with it when consensus is reached :) There are approx. ~90 of these types. Not all of them are added to their respective module dicts though, but I would add the flag nonetheless, as it simplifies creating and reviewing the patch, and it does not have any performance impact.

[erlend-aasland]

BTW, slightly related, AFAICT:

• cpython/Objects/typeobject.c

  Lines 4620 to 4668 in e047239

  	/* In versions of CPython prior to 3.5, the code in
  	compatible_for_assignment was not set up to correctly check for memory
  	layout / slot / etc. compatibility for non-HEAPTYPE classes, so we just
  	disallowed __class__ assignment in any case that wasn't HEAPTYPE ->
  	HEAPTYPE.
  	During the 3.5 development cycle, we fixed the code in
  	compatible_for_assignment to correctly check compatibility between
  	arbitrary types, and started allowing __class__ assignment in all cases
  	where the old and new types did in fact have compatible slots and
  	memory layout (regardless of whether they were implemented as HEAPTYPEs
  	or not).
  	Just before 3.5 was released, though, we discovered that this led to
  	problems with immutable types like int, where the interpreter assumes
  	they are immutable and interns some values. Formerly this wasn't a
  	problem, because they really were immutable -- in particular, all the
  	types where the interpreter applied this interning trick happened to
  	also be statically allocated, so the old HEAPTYPE rules were
  	"accidentally" stopping them from allowing __class__ assignment. But
  	with the changes to __class__ assignment, we started allowing code like
  	class MyInt(int):
  	...
  	# Modifies the type of *all* instances of 1 in the whole program,
  	# including future instances (!), because the 1 object is interned.
  	(1).__class__ = MyInt
  	(see https://bugs.python.org/issue24912).
  	In theory the proper fix would be to identify which classes rely on
  	this invariant and somehow disallow __class__ assignment only for them,
  	perhaps via some mechanism like a new Py_TPFLAGS_IMMUTABLE flag (a
  	"denylisting" approach). But in practice, since this problem wasn't
  	noticed late in the 3.5 RC cycle, we're taking the conservative
  	approach and reinstating the same HEAPTYPE->HEAPTYPE check that we used
  	to have, plus an "allowlist". For now, the allowlist consists only of
  	ModuleType subtypes, since those are the cases that motivated the patch
  	in the first place -- see https://bugs.python.org/issue22986 -- and
  	since module objects are mutable we can be sure that they are
  	definitely not being interned. So now we allow HEAPTYPE->HEAPTYPE *or*
  	ModuleType subtype -> ModuleType subtype.
  	So far as we know, all the code beyond the following 'if' statement
  	will correctly handle non-HEAPTYPE classes, and the HEAPTYPE check is
  	needed only to protect that subset of non-HEAPTYPE classes for which
  	the interpreter has baked in the assumption that all instances are
  	truly immutable.
  	*/

• bpo-24912

[erlend-aasland]

I've prepared a diff using grep & sed (see attached patch). Let me know if you want it converted to a PR.

I've excluded the test and the _xx extension modules.

[gvanrossum]

Does the test suite pass for apply-to-all.diff?

Also, quite a hornet's nest you've uncovered (about __class__ assignment). Would that be fixed better with the IMMUTABLE flag?

[erlend-aasland]

Does the test suite pass for apply-to-all.diff?

No, multiple tests fail. First test_distutils fails, then during the re-run, test_multiprocessing_forkserver, test_multiprocessing_spawn, and test_pdb fail.

Also, quite a hornet's nest you've uncovered (about __class__ assignment).

Yes, indeed. Apropos, I see that bpo-24991 is still open.

Would that be fixed better with the IMMUTABLE flag?

That would be a change of functionality, given that we apply the immutable flag to all built-in types. Currently __class__ assignment is allowed for heap types, and it has been so for many years.

I feel reluctant to add apply-to-all.diff to the PR right now.

[erlend-aasland]

The more I read about these issues, the more I feel reluctant to apply PR 25520. Why change the behaviour/traits of heap types without a proper discussion (or PEP)? Applying this PR feels like an arbitrary change. All the related issues and mailing list discussions seems to end without consensus, and fundamental questions (what is an immutable type? which types should be immutable? why is it a problem that a type is mutable?) seems to be avoided all the time.

And, as Victor points out in msg391596 (and which has also been pointed out by others in other discussions):

Currently, the limitation of not being able to modify a built-in type is arbitrary, it's not a technical limitation

FWIW #1, in pypy3, datetime.timedelta.foo = 1 is ok; in python3 (3.7-3.10) it's not.

FWIW #2, in Python 3.9, functools.partialmethod is implemented in Python, and functools.partial is implemented in C as a static type:
$ python3.9
>>> import functools
>>> functools.partial.foo = 1  # is this a problem?
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: can't set attributes of built-in/extension type 'functools.partial'
>>> functools.partialmethod.foo = 1  # is this a problem?

If I'm only bringing noise into the discussion, feel free to remove me from the nosy list.

[pablogsal]

Notice this issue is marked as "deferred blocker" it won't block this beta release (beta 2) but it will block the next.

[erlend-aasland]

PR 26351 adds the Py_TPFLAGS_IMMUTABLETYPE type flag to all types converted to heap type during 3.10 development.

[pablogsal]

New changeset 00710e6 by Erlend Egeberg Aasland in branch 'main':
bpo-43908: Make heap types converted during 3.10 alpha immutable (GH-26351)
00710e6

[pablogsal]

New changeset 7297d74 by Miss Islington (bot) in branch '3.10':
bpo-43908: Make heap types converted during 3.10 alpha immutable (GH-26351) (GH-26766)
7297d74

[erlend-aasland]

In inherit_slots() in Objects/typeobject.c, Py_TPFLAGS_HAVE_VECTORCALL inheritance depends on if the base type is a heap type or not. This aligns with PEP-590[1]:

Heap types never inherit the vectorcall protocol because that
would not be safe (heap types can be changed dynamically).

AFAICS, inherit_slots() should now use Py_TPFLAGS_IMMUTABLETYPE to decide if Py_TPFLAGS_HAVE_VECTORCALL can be inherited, and the PEP should be updated.

• [1] https://www.python.org/dev/peps/pep-0590/#subclassing

[gvanrossum]

+1--
--Guido (mobile)

[vstinner]

New changeset a3739b2 by Erlend Egeberg Aasland in branch 'main':
bpo-43908: Immutable types inherit vectorcall (GH-27001)
a3739b2

[pablogsal]

Is anything left here?

[erlend-aasland]

Is anything left here?

Should the immutable flag also be applied to the heap types converted in and before Python 3.9 before closing this issue?

[pablogsal]

I would say yes. Do you have a compiled list of what's left?

[erlend-aasland]

These types now lack the Py_TPFLAGS_IMMUTABLETYPE flag:

## Types converted in Python 3.9

• _abc._abc_data
• _json.Encoder
• _json.Scanner
• _random.Random
• _struct.Struct
• _struct.unpack_iterator
• ast.AST
• posix.DirEntry
• posix.ScandirIterator
• select.devpoll
• select.epoll
• select.kevent
• select.kqueue
• select.poll
• zlib.Compress
• zlib.Decompress

## Types converted before Python 3.9

• _curses_panel.panel
• _tkinter.Tcl_Obj
• _tkinter.tkapp
• _tkinter.tktimertoken

[pablogsal]

Yeah, I think these should be changed, but lest give a bit of time for other core devs to mention what they think

[vstinner]

Should the immutable flag also be applied to the heap types converted in and before Python 3.9 before closing this issue?

I don't think that Python 3.9 should be changed. It's too late. IMO this issue is not important enough to introduce a new type flag in a minor Python 3.9.x bugfix release. I suggest to close this issue.

[vstinner]

In Python 3.11, 68 heap types have the Py_TPFLAGS_IMMUTABLETYPE flag:

• _blake2.blake2b
• _blake2.blake2s
• _bz2.BZ2Compressor
• _bz2.BZ2Decompressor
• _csv.Dialect
• _csv.reader
• _csv.writer
• _dbm.dbm
• _gdbm.gdbm
• _hashlib.HASH
• _hashlib.HASHXOF
• _hashlib.HMAC
• _lsprof.Profiler
• _lzma.LZMACompressor
• _lzma.LZMADecompressor
• _md5.md5
• _multibytecodec.MultibyteCodec
• _multibytecodec.MultibyteIncrementalDecoder
• _multibytecodec.MultibyteIncrementalEncoder
• _multibytecodec.MultibyteStreamReader
• _multibytecodec.MultibyteStreamWriter
• _overlapped.Overlapped
• _queue.SimpleQueue
• _sha1.sha1
• _sha256.sha224
• _sha256.sha256
• _sha3.keccak_224
• _sha3.keccak_256
• _sha3.keccak_384
• _sha3.keccak_512
• _sha3.sha3_224
• _sha3.sha3_256
• _sha3.sha3_384
• _sha3.sha3_512
• _sha512.sha384
• _sha512.sha512
• _sre.SRE_Scanner
• _ssl.Certificate
• _ssl.MemoryBIO
• _ssl.SSLSession
• _ssl._SSLContext
• _ssl._SSLSocket
• ssl.SSLError
• _thread.RLock
• _thread._local
• _thread._localdummy
• _thread.lock
• _tokenize.TokenizerIter
• _winapi.Overlapped
• array.array
• array.arrayiterator
• functools.KeyWrapper
• functools._lru_cache_wrapper
• functools._lru_list_elem
• functools.partial
• mmap.mmap
• operator.attrgetter
• operator.itemgetter
• operator.methodcaller
• pyexpat.xmlparser
• re.Match
• re.Pattern
• sqlite3.Connection
• sqlite3.Cursor
• sqlite3.PrepareProtocol
• sqlite3.Row
• sqlite3.Statement
• unicodedata.UCD

[erlend-aasland]

Should the immutable flag also be applied to the heap types converted in and before Python 3.9 before closing this issue?

I don't think that Python 3.9 should be changed. It's too late. IMO this issue is not important enough to introduce a new type flag in a minor Python 3.9.x bugfix release. I suggest to close this issue.

3.9 has now entered security-only mode; suggesting to close this issue.

[vstinner]

Right, I closed the issue.

[gpshead]

FYI a question related to this new flag has arisen: https://discuss.python.org/t/immutable-types-with-mutable-bases/17764