Address CodeRabbit concerns: fix GC clearing and improve thread safety of lazy __dict__ recreation

alok-108 · alok-108 · commit 28c5fe2c2b7a · 2026-04-07T03:02:44.000+05:30
diff --git a/crates/vm/src/builtins/object.rs b/crates/vm/src/builtins/object.rs
@@ -558,11 +558,7 @@ pub fn object_get_dict(obj: PyObjectRef, vm: &VirtualMachine) -> PyResult<PyDict
         Ok(dict)
     } else {
         match obj.instance_dict() {
-            Some(d) => {
-                let dict = vm.ctx.new_dict();
-                d.set(Some(dict.clone()));
-                Ok(dict)
-            }
+            Some(d) => Ok(d.get_or_insert(vm)),
             None => Err(vm.new_attribute_error("This object has no __dict__")),
         }
     }
diff --git a/crates/vm/src/object/core.rs b/crates/vm/src/object/core.rs
@@ -909,8 +909,8 @@ impl Py<PyWeak> {
 }
 
 #[derive(Debug)]
-pub(super) struct InstanceDict {
-    pub(super) d: PyRwLock<Option<PyDictRef>>,
+pub(crate) struct InstanceDict {
+    pub(crate) d: PyRwLock<Option<PyDictRef>>,
 }
 
 impl From<PyDictRef> for InstanceDict {
@@ -948,6 +948,17 @@ impl InstanceDict {
     pub fn into_inner(self) -> Option<PyDictRef> {
         self.d.into_inner()
     }
+
+    pub(crate) fn get_or_insert(&self, vm: &VirtualMachine) -> PyDictRef {
+        let mut d = self.d.write();
+        if let Some(existing) = d.as_ref() {
+            existing.clone()
+        } else {
+            let dict = vm.ctx.new_dict();
+            *d = Some(dict.clone());
+            dict
+        }
+    }
 }
 
 impl<T: PyPayload> PyInner<T> {
@@ -1776,8 +1787,9 @@ impl PyObject {
             let ext = unsafe { &mut *ext_ptr };
             if let Some(old_dict) = ext.dict.take() {
                 // Get the dict ref before dropping InstanceDict
-                let dict_ref = old_dict.into_inner();
-                result.push(dict_ref.into());
+                if let Some(dict_ref) = old_dict.into_inner() {
+                    result.push(dict_ref.into());
+                }
             }
             for slot in ext.slots.iter() {
                 if let Some(val) = slot.write().take() {

Original file line number	Diff line number	Diff line change
`@@ -558,11 +558,7 @@ pub fn object_get_dict(obj: PyObjectRef, vm: &VirtualMachine) -> PyResult<PyDict`
`558`	`558`	`Ok(dict)`
`559`	`559`	`} else {`
`560`	`560`	`match obj.instance_dict() {`
`561`		`- Some(d) => {`
`562`		`- let dict = vm.ctx.new_dict();`
`563`		`- d.set(Some(dict.clone()));`
`564`		`- Ok(dict)`
`565`		`- }`
	`561`	`+ Some(d) => Ok(d.get_or_insert(vm)),`
`566`	`562`	`None => Err(vm.new_attribute_error("This object has no __dict__")),`
`567`	`563`	`}`
`568`	`564`	`}`