Skip to content

Commit 12dfd4d

Browse files
authored
gh-159434: Avoid borrowed references in func_get_annotation_dict() (gh-159436)
Hold strong references to __annotate__ and the annotations tuple while running code that can replace them. If __annotations__ is set while __annotate__ runs, keep the newly set value.
1 parent 41c224c commit 12dfd4d

3 files changed

Lines changed: 37 additions & 3 deletions

File tree

‎Lib/test/test_type_annotations.py‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -328,6 +328,18 @@ def check_annotations(self, f):
328328
f.__annotations__ = {"z": 43}
329329
self.assertIs(f.__annotate__, None)
330330

331+
def test_annotate_sets_annotations(self):
332+
# If __annotate__ sets the function's annotations while it runs,
333+
# those are kept instead of the result of the call.
334+
def f():
335+
pass
336+
def annotate(format):
337+
f.__annotations__ = {"x": int}
338+
return {"y": str}
339+
f.__annotate__ = annotate
340+
self.assertEqual(f.__annotations__, {"x": int})
341+
self.assertIs(f.__annotate__, None)
342+
331343
def test_user_defined_annotate(self):
332344
class X:
333345
a: int
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
Fix reading a function's :attr:`~function.__annotations__` using borrowed
2+
references to :attr:`~function.__annotate__` and the internal annotations
3+
tuple while running code that can replace them. If ``__annotations__`` is
4+
set while ``__annotate__`` runs, the newly set value is now kept.

‎Objects/funcobject.c‎

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -560,11 +560,18 @@ func_get_annotation_dict(PyFunctionObject *op)
560560
{
561561
_Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(op);
562562
if (op->func_annotations == NULL) {
563-
if (op->func_annotate == NULL || !PyCallable_Check(op->func_annotate)) {
563+
PyObject *annotate = op->func_annotate;
564+
if (annotate == NULL || !PyCallable_Check(annotate)) {
564565
Py_RETURN_NONE;
565566
}
567+
// Calling __annotate__ can run arbitrary code which may replace
568+
// func_annotate or func_annotations, either directly or from another
569+
// thread while the critical section is suspended. Keep the callable
570+
// alive during the call.
571+
Py_INCREF(annotate);
566572
PyObject *one = _PyLong_GetOne();
567-
PyObject *ann_dict = _PyObject_CallOneArg(op->func_annotate, one);
573+
PyObject *ann_dict = _PyObject_CallOneArg(annotate, one);
574+
Py_DECREF(annotate);
568575
if (ann_dict == NULL) {
569576
return NULL;
570577
}
@@ -575,13 +582,22 @@ func_get_annotation_dict(PyFunctionObject *op)
575582
Py_DECREF(ann_dict);
576583
return NULL;
577584
}
585+
if (op->func_annotations != NULL) {
586+
// Annotations were set during the call, keep them.
587+
Py_DECREF(ann_dict);
588+
assert(PyDict_Check(op->func_annotations));
589+
return op->func_annotations;
590+
}
578591
_PyObject_XSetRefDelayed(&op->func_annotations, ann_dict);
579592
return ann_dict;
580593
}
581594
if (PyTuple_CheckExact(op->func_annotations)) {
582-
PyObject *ann_tuple = op->func_annotations;
595+
// Keep the tuple alive while building the dict, since it can be
596+
// replaced if the critical section is suspended.
597+
PyObject *ann_tuple = Py_NewRef(op->func_annotations);
583598
PyObject *ann_dict = PyDict_New();
584599
if (ann_dict == NULL) {
600+
Py_DECREF(ann_tuple);
585601
return NULL;
586602
}
587603

@@ -594,10 +610,12 @@ func_get_annotation_dict(PyFunctionObject *op)
594610

595611
if (err < 0) {
596612
Py_DECREF(ann_dict);
613+
Py_DECREF(ann_tuple);
597614
return NULL;
598615
}
599616
}
600617
_PyObject_XSetRefDelayed(&op->func_annotations, ann_dict);
618+
_PyObject_XDecRefDelayed(ann_tuple);
601619
}
602620
assert(PyDict_Check(op->func_annotations));
603621
return op->func_annotations;

0 commit comments

Comments
 (0)