Fix race in constructing dict for instance

DinoV · DinoV · commit ae6d925e9708 · 2024-05-01T19:48:39.000-07:00
diff --git a/Include/internal/pycore_dict.h b/Include/internal/pycore_dict.h
@@ -105,7 +105,7 @@ PyAPI_FUNC(PyObject *)_PyDict_LoadGlobal(PyDictObject *, PyDictObject *, PyObjec
 
 /* Consumes references to key and value */
 PyAPI_FUNC(int) _PyDict_SetItem_Take2(PyDictObject *op, PyObject *key, PyObject *value);
-extern int _PyObjectDict_SetItem(PyTypeObject *tp, PyObject **dictptr, PyObject *name, PyObject *value);
+extern int _PyObjectDict_SetItem(PyTypeObject *tp, PyObject *obj, PyObject **dictptr, PyObject *name, PyObject *value);
 
 extern int _PyDict_Pop_KnownHash(
     PyDictObject *dict,
diff --git a/Objects/dictobject.c b/Objects/dictobject.c
@@ -7213,7 +7213,7 @@ PyObject_GenericGetDict(PyObject *obj, void *context)
 }
 
 int
-_PyObjectDict_SetItem(PyTypeObject *tp, PyObject **dictptr,
+_PyObjectDict_SetItem(PyTypeObject *tp, PyObject *obj, PyObject **dictptr,
                       PyObject *key, PyObject *value)
 {
     PyObject *dict;
@@ -7222,37 +7222,39 @@ _PyObjectDict_SetItem(PyTypeObject *tp, PyObject **dictptr,
     PyInterpreterState *interp = _PyInterpreterState_GET();
 
     assert(dictptr != NULL);
-    if ((tp->tp_flags & Py_TPFLAGS_HEAPTYPE) && (cached = CACHED_KEYS(tp))) {
-        assert(dictptr != NULL);
+    dict = *dictptr;
+    if (dict == NULL) {
+#ifdef Py_GIL_DISABLED
+        Py_BEGIN_CRITICAL_SECTION(obj);
         dict = *dictptr;
-        if (dict == NULL) {
+        if (dict != NULL) {
+            goto done;
+        }
+#endif
+        if ((tp->tp_flags & Py_TPFLAGS_HEAPTYPE) && (cached = CACHED_KEYS(tp))) {
             assert(!_PyType_HasFeature(tp, Py_TPFLAGS_INLINE_VALUES));
             dictkeys_incref(cached);
             dict = new_dict_with_shared_keys(interp, cached);
-            if (dict == NULL)
-                return -1;
-            *dictptr = dict;
-        }
-        if (value == NULL) {
-            res = PyDict_DelItem(dict, key);
+            if (dict == NULL) {
+                dictkeys_decref(interp, cached, false);
+            }
         }
         else {
-            res = PyDict_SetItem(dict, key, value);
-        }
-    } else {
-        dict = *dictptr;
-        if (dict == NULL) {
             dict = PyDict_New();
-            if (dict == NULL)
-                return -1;
-            *dictptr = dict;
         }
-        if (value == NULL) {
-            res = PyDict_DelItem(dict, key);
-        } else {
-            res = PyDict_SetItem(dict, key, value);
+        *dictptr = dict;
+#ifdef Py_GIL_DISABLED
+done:
+        Py_END_CRITICAL_SECTION();
+#endif
+        if (dict == NULL) {
+            return -1;
         }
     }
+
+    Py_BEGIN_CRITICAL_SECTION(dict);
+    res = set_or_del_lock_held((PyDictObject *)dict, key, value);
+    Py_END_CRITICAL_SECTION();
     ASSERT_CONSISTENT(dict);
     return res;
 }
diff --git a/Objects/object.c b/Objects/object.c
@@ -1732,7 +1732,7 @@ _PyObject_GenericSetAttrWithDict(PyObject *obj, PyObject *name,
             goto done;
         }
         else {
-            res = _PyObjectDict_SetItem(tp, dictptr, name, value);
+            res = _PyObjectDict_SetItem(tp, obj, dictptr, name, value);
         }
     }
     else {

Original file line number	Diff line number	Diff line change
`@@ -1732,7 +1732,7 @@ _PyObject_GenericSetAttrWithDict(PyObject obj, PyObject name,`
`1732`	`1732`	`goto done;`
`1733`	`1733`	`}`
`1734`	`1734`	`else {`
`1735`		`- res = _PyObjectDict_SetItem(tp, dictptr, name, value);`
	`1735`	`+ res = _PyObjectDict_SetItem(tp, obj, dictptr, name, value);`
`1736`	`1736`	`}`
`1737`	`1737`	`}`
`1738`	`1738`	`else {`