Skip to content

Commit 4aaa512

Browse files
naschememiss-islington
authored andcommitted
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. (cherry picked from commit 12dfd4d) Co-authored-by: Neil Schemenauer <nas-github@arctrix.com>
1 parent f3f0b0a commit 4aaa512

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
@@ -577,11 +577,18 @@ func_get_annotation_dict(PyFunctionObject *op)
577577
{
578578
_Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(op);
579579
if (op->func_annotations == NULL) {
580-
if (op->func_annotate == NULL || !PyCallable_Check(op->func_annotate)) {
580+
PyObject *annotate = op->func_annotate;
581+
if (annotate == NULL || !PyCallable_Check(annotate)) {
581582
Py_RETURN_NONE;
582583
}
584+
// Calling __annotate__ can run arbitrary code which may replace
585+
// func_annotate or func_annotations, either directly or from another
586+
// thread while the critical section is suspended. Keep the callable
587+
// alive during the call.
588+
Py_INCREF(annotate);
583589
PyObject *one = _PyLong_GetOne();
584-
PyObject *ann_dict = _PyObject_CallOneArg(op->func_annotate, one);
590+
PyObject *ann_dict = _PyObject_CallOneArg(annotate, one);
591+
Py_DECREF(annotate);
585592
if (ann_dict == NULL) {
586593
return NULL;
587594
}
@@ -591,13 +598,22 @@ func_get_annotation_dict(PyFunctionObject *op)
591598
Py_DECREF(ann_dict);
592599
return NULL;
593600
}
601+
if (op->func_annotations != NULL) {
602+
// Annotations were set during the call, keep them.
603+
Py_DECREF(ann_dict);
604+
assert(PyDict_Check(op->func_annotations));
605+
return op->func_annotations;
606+
}
594607
_PyObject_XSetRefDelayed(&op->func_annotations, ann_dict);
595608
return ann_dict;
596609
}
597610
if (PyTuple_CheckExact(op->func_annotations)) {
598-
PyObject *ann_tuple = op->func_annotations;
611+
// Keep the tuple alive while building the dict, since it can be
612+
// replaced if the critical section is suspended.
613+
PyObject *ann_tuple = Py_NewRef(op->func_annotations);
599614
PyObject *ann_dict = PyDict_New();
600615
if (ann_dict == NULL) {
616+
Py_DECREF(ann_tuple);
601617
return NULL;
602618
}
603619

@@ -610,10 +626,12 @@ func_get_annotation_dict(PyFunctionObject *op)
610626

611627
if (err < 0) {
612628
Py_DECREF(ann_dict);
629+
Py_DECREF(ann_tuple);
613630
return NULL;
614631
}
615632
}
616633
_PyObject_XSetRefDelayed(&op->func_annotations, ann_dict);
634+
_PyObject_XDecRefDelayed(ann_tuple);
617635
}
618636
assert(PyDict_Check(op->func_annotations));
619637
return op->func_annotations;

0 commit comments

Comments
 (0)