gh-158198: Fix crash when de-inlining dict while freeing owning object (#160114)

* gh-158198: Fix crash when de-inlining dict while freeing owning object

Author: DinoV

Lib/test/test_dict.py

@@ -1835,6 +1835,45 @@ def __del__(self):
         d = x.__dict__
         d.clear()
 
+    def test_clear_reentrant_detached_split(self):
+        # gh-158198 follow-up: dict.clear() on a detached split-table dict
+        # must not expose (Py_EMPTY_KEYS, non-NULL ma_values) to a
+        # finalizer.  A reentrant insert must see a consistent empty dict.
+        class MyObj: pass
+        class Reinsert:
+            def __del__(self):
+                d['reentrant'] = 1
+
+        x = MyObj()
+        x.a = Reinsert()
+        x.b = 'other'
+
+        d = x.__dict__
+        # Detach: the old dict becomes a heap-allocated split table
+        # (ma_values non-NULL, embedded == 0).
+        x.__dict__ = {}
+        d.clear()
+        self.assertEqual(d, {'reentrant': 1})
+
+    def test_clear_reentrant_detached_split_update(self):
+        # Same as above but with a reentrant update large enough to force
+        # a resize.  On the buggy code this frees the values array that
+        # clear() is still iterating over (use-after-free / segfault).
+        class MyObj: pass
+        other = {f'k{i}': i for i in range(20)}
+        class Reinsert:
+            def __del__(self):
+                d.update(other)
+
+        x = MyObj()
+        x.a = Reinsert()
+        x.b = 'other'
+
+        d = x.__dict__
+        x.__dict__ = {}
+        d.clear()
+        self.assertEqual(d, other)
+
 
 class CAPITest(unittest.TestCase):
 

Lib/test/test_free_threading/test_dict.py

@@ -5,7 +5,7 @@
 
 from ast import Or
 from functools import partial
-from threading import Barrier, Thread
+from threading import Barrier, Event, Thread
 from unittest import TestCase
 
 try:
@@ -437,6 +437,45 @@ def writer():
 
         threading_helper.run_concurrently([reader, writer])
 
+    def test_racing_dealloc_and_split_dict_resize(self):
+        # gh-158198: an object's inline-values __dict__ escapes to another
+        # thread, which converts it to a combined table (by adding a non-str
+        # key) while the owning thread drops the last reference to the object.
+        NATTRS = 20
+
+        class C:
+            def __init__(self):
+                for i in range(NATTRS):
+                    setattr(self, f"a{i}", i)
+
+        def make_pair():
+            slot = [None]
+            done = Event()
+
+            def owner():
+                try:
+                    for j in range(2000):
+                        o = C()
+                        for i in range(NATTRS):
+                            self.assertEqual(getattr(o, f"a{i}"), i)
+                        slot[0] = o.__dict__
+                        for _ in range(j % 16):
+                            pass
+                        del o
+                finally:
+                    done.set()
+
+            def converter():
+                while not done.is_set():
+                    d = slot[0]
+                    if d is not None:
+                        d[0] = 0
+                        slot[0] = None
+
+            return [owner, converter]
+
+        threading_helper.run_concurrently(make_pair() + make_pair())
+
     def test_racing_dict_update_and_method_lookup_with_inline_values(self):
         # gh-149816: sub-case 108
         # The race below checks that a detached dict is still valid

Objects/dictobject.c

@@ -2276,7 +2276,6 @@ dictresize(PyDictObject *mp,
         UNLOCK_KEYS(oldkeys);
         set_keys(mp, newkeys);
         dictkeys_decref(oldkeys, IS_DICT_SHARED(mp));
-        set_values(mp, NULL);
         if (oldvalues->embedded) {
             assert(oldvalues->embedded == 1);
             assert(oldvalues->valid == 1);
@@ -2285,6 +2284,7 @@ dictresize(PyDictObject *mp,
         else {
             free_values(oldvalues, IS_DICT_SHARED(mp));
         }
+        set_values(mp, NULL);
     }
     else {  // oldkeys is combined.
         if (oldkeys->dk_kind == DICT_KEYS_GENERAL) {
@@ -8024,7 +8024,7 @@ PyObject_ClearManagedDict(PyObject *obj)
             clear_inline_values(_PyObject_InlineValues(obj));
             return;
         }
-        else if (FT_ATOMIC_LOAD_PTR_RELAXED(dict->ma_values) ==
+        if (FT_ATOMIC_LOAD_PTR_ACQUIRE(dict->ma_values) ==
                     _PyObject_InlineValues(obj)) {
             // We have a materialized object which points at the inline
             // values. We need to materialize the keys. Nothing can modify