Skip to content

Commit 04afc4d

Browse files
committed
Copy str subclass keys of the class namespace to exact str.
type_new() now copies str subclass keys of the class namespace to exact str, as type_setattro() already does for assigned names. Lookups of a slot name in a type dict then cannot call a user __eq__ through a str subclass key. Non-str keys are still allowed (with a RuntimeWarning) and can still run __eq__; the type_update_dict() comment is corrected to say so. Remove the free-threading tests that relied on str subclass keys in the type dict.
1 parent 8b8a87c commit 04afc4d

3 files changed

Lines changed: 93 additions & 75 deletions

File tree

‎Lib/test/test_descr.py‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5080,6 +5080,31 @@ def __new__(mcls, name, bases, ns):
50805080
with self.assertWarnsRegex(RuntimeWarning, 'MyClass'):
50815081
MyClass = meta('MyClass', (), {})
50825082

5083+
def test_str_subclass_namespace_keys(self):
5084+
# Keys of the class namespace that are str subclasses are copied
5085+
# to exact str in the type's __dict__, like type.__setattr__ does.
5086+
class Key(str):
5087+
def __hash__(self):
5088+
# Collide with the slot name assigned below.
5089+
return hash('__hash__')
5090+
def __eq__(self, other):
5091+
# Must not be called by type dict lookups.
5092+
raise AssertionError('Key.__eq__ called')
5093+
5094+
ns = {Key('other'): 1, '__repr__': lambda self: 'r', 'c': 2}
5095+
C = type('C', (), ns)
5096+
for key in C.__dict__:
5097+
self.assertIs(type(key), str)
5098+
self.assertEqual([str(k) for k in ns], list(C.__dict__)[:3])
5099+
self.assertEqual(repr(C()), 'r')
5100+
self.assertEqual(C.other, 1)
5101+
# Lookups and slot updates that hash-collide with 'other' do not
5102+
# call Key.__eq__.
5103+
C.__hash__ = None
5104+
with self.assertRaises(TypeError):
5105+
hash(C())
5106+
del C.__hash__
5107+
50835108
def test_proxy_call(self):
50845109
class FakeStr:
50855110
__class__ = str

‎Lib/test/test_free_threading/test_type.py‎

Lines changed: 0 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import gc
22
import threading
33
import unittest
4-
import weakref
54

65
from concurrent.futures import ThreadPoolExecutor
76
from threading import Barrier, Thread
@@ -470,76 +469,6 @@ def setter():
470469

471470
self.check_stop_the_world_under_type_lock(setter)
472471

473-
def test_stop_the_world_in_setattr_key_eq_deadlock(self):
474-
# type_setattro() compares the attribute name with a str subclass key
475-
# of the type's __dict__ (calling its __eq__) with the type lock held.
476-
N = 50
477-
478-
class S(str):
479-
__hash__ = str.__hash__
480-
def __eq__(self, other):
481-
_testinternalcapi.stop_the_world()
482-
return str.__eq__(self, other)
483-
484-
C = type("C", (), {S("x"): 0})
485-
486-
def setter():
487-
for i in range(N):
488-
C.x = i
489-
490-
self.check_stop_the_world_under_type_lock(setter)
491-
492-
def test_reentrant_slot_update_consistency(self):
493-
# Updating Base queues its slot update before visiting Sub. A key
494-
# comparison in Sub's dict re-enters the setter and makes that queue
495-
# stale. It must be rebuilt rather than overwriting the inner update.
496-
armed = False
497-
498-
class Base:
499-
pass
500-
501-
class Key(str):
502-
def __hash__(self):
503-
return hash("__hash__")
504-
505-
def __eq__(self, other):
506-
nonlocal armed
507-
if armed and other == "__hash__":
508-
armed = False
509-
Base.__hash__ = object.__hash__
510-
return str.__eq__(self, other)
511-
512-
# Collide with the special-method name without defining it on Sub,
513-
# so that slot updates still propagate from Base to Sub.
514-
Sub = type("Sub", (Base,), {Key("trigger"): None})
515-
objects = [Base(), Sub()]
516-
517-
# Check actual slot dispatch, not just the class dictionaries. None
518-
# and object.__hash__ use distinct C slots, unlike Python functions
519-
# which share a generic wrapper that looks up the current method.
520-
Base.__hash__ = None
521-
for obj in objects:
522-
with self.assertRaises(TypeError):
523-
hash(obj)
524-
Base.__hash__ = object.__hash__
525-
for obj in objects:
526-
self.assertEqual(hash(obj), object.__hash__(obj))
527-
528-
armed = True
529-
Base.__hash__ = None
530-
self.assertFalse(armed)
531-
self.assertIs(Base.__dict__["__hash__"], object.__hash__)
532-
for obj in objects:
533-
self.assertEqual(hash(obj), object.__hash__(obj))
534-
535-
# Both applied and discarded queues must release their references.
536-
base_ref = weakref.ref(Base)
537-
sub_ref = weakref.ref(Sub)
538-
del obj, objects, Sub, Base
539-
gc.collect()
540-
self.assertIsNone(base_ref())
541-
self.assertIsNone(sub_ref())
542-
543472
def test_reentrant_mro_always_publishes(self):
544473
# A custom mro() that updates a slot on every call must not make
545474
# the __bases__ assignment retry forever.

‎Objects/typeobject.c‎

Lines changed: 68 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4978,10 +4978,72 @@ type_new_get_slots(type_new_ctx *ctx, PyObject *dict)
49784978
}
49794979

49804980

4981+
/* Copy the class namespace. Keys that are str subclasses are replaced by
4982+
exact str copies so that the type dict only holds exact str keys (see
4983+
type_setattro() which does the same for assigned names). Lookups in a
4984+
dict with exact str keys cannot run user __eq__ methods, which the slot
4985+
update code relies on. Non-str keys are left alone; type_new_impl()
4986+
warns about them. */
4987+
static PyObject *
4988+
type_new_copy_dict(PyObject *orig_dict)
4989+
{
4990+
PyObject *dict = _PyDict_CopyAsDict(orig_dict);
4991+
if (dict == NULL) {
4992+
return NULL;
4993+
}
4994+
if (((PyDictObject *)dict)->ma_keys->dk_kind != DICT_KEYS_GENERAL) {
4995+
// Only exact str keys are possible, nothing to do.
4996+
return dict;
4997+
}
4998+
Py_ssize_t pos = 0;
4999+
PyObject *key, *value;
5000+
int rebuild = 0;
5001+
while (PyDict_Next(dict, &pos, &key, &value)) {
5002+
if (PyUnicode_Check(key) && !PyUnicode_CheckExact(key)) {
5003+
rebuild = 1;
5004+
break;
5005+
}
5006+
}
5007+
if (!rebuild) {
5008+
return dict;
5009+
}
5010+
// Rebuild rather than delete/insert so that the key order is kept.
5011+
PyObject *newdict = PyDict_New();
5012+
if (newdict == NULL) {
5013+
Py_DECREF(dict);
5014+
return NULL;
5015+
}
5016+
pos = 0;
5017+
while (PyDict_Next(dict, &pos, &key, &value)) {
5018+
PyObject *newkey;
5019+
if (PyUnicode_Check(key) && !PyUnicode_CheckExact(key)) {
5020+
newkey = _PyUnicode_Copy(key);
5021+
if (newkey == NULL) {
5022+
goto error;
5023+
}
5024+
}
5025+
else {
5026+
newkey = Py_NewRef(key);
5027+
}
5028+
int err = PyDict_SetItem(newdict, newkey, value);
5029+
Py_DECREF(newkey);
5030+
if (err < 0) {
5031+
goto error;
5032+
}
5033+
}
5034+
Py_DECREF(dict);
5035+
return newdict;
5036+
5037+
error:
5038+
Py_DECREF(dict);
5039+
Py_DECREF(newdict);
5040+
return NULL;
5041+
}
5042+
49815043
static PyTypeObject*
49825044
type_new_init(type_new_ctx *ctx)
49835045
{
4984-
PyObject *dict = _PyDict_CopyAsDict(ctx->orig_dict);
5046+
PyObject *dict = type_new_copy_dict(ctx->orig_dict);
49855047
if (dict == NULL) {
49865048
goto error;
49875049
}
@@ -6673,9 +6735,11 @@ type_update_dict(PyTypeObject *type, PyDictObject *dict, PyObject *name,
66736735
// We don't want any re-entrancy between when we update the dict
66746736
// and call _PyType_Modified_Unlocked, including running the destructor
66756737
// of the current value as it can observe the cache in an inconsistent
6676-
// state. Because we have an exact unicode and our dict has exact
6677-
// unicodes we know that this will all complete without releasing
6678-
// the locks.
6738+
// state. The name is an exact unicode (see type_setattro()) and
6739+
// str subclass keys are copied to exact str by type_new_copy_dict()
6740+
// and type_setattro(). The type dict can still hold non-str keys
6741+
// (type_new_impl() only warns about them) and comparing with one of
6742+
// those can run user code that releases the locks.
66796743
if (_PyDict_GetItemRef_Unicode_LockHeld(dict, name, old_value) < 0) {
66806744
return -1;
66816745
}

0 commit comments

Comments
 (0)