Skip to content

Commit 0e55bc9

Browse files
coolreader18coolreader18
committed
Make PyObject.dict private and a Mutex; add some TODOs
1 parent a9e784c commit 0e55bc9

File tree

7 files changed

+48
-40
lines changed

7 files changed

+48
-40
lines changed

‎vm/src/builtins.rs

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -261,14 +261,7 @@ fn make_scope(vm: &VirtualMachine, scope: ScopeArgs) -> PyResult<Scope> {
261261
let globals = match globals {
262262
Some(dict) => {
263263
if !dict.contains_key("__builtins__", vm) {
264-
let builtins_dict = vm
265-
.builtins
266-
.dict
267-
.as_ref()
268-
.unwrap()
269-
.borrow()
270-
.as_object()
271-
.clone();
264+
let builtins_dict = vm.builtins.dict().unwrap().as_object().clone();
272265
dict.set_item("__builtins__", builtins_dict, vm).unwrap();
273266
}
274267
dict

‎vm/src/frame.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -701,8 +701,8 @@ impl Frame {
701701
let module = self.pop_value();
702702

703703
// Grab all the names from the module and put them in the context
704-
if let Some(dict) = &module.dict {
705-
for (k, v) in &*dict.borrow() {
704+
if let Some(dict) = module.dict() {
705+
for (k, v) in &dict {
706706
let k = vm.to_str(&k)?;
707707
let k = k.as_str();
708708
if !k.starts_with('_') {

‎vm/src/function.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -459,6 +459,7 @@ pub type FunctionBox<T> = SmallBox<T, S1>;
459459

460460
/// A built-in Python function.
461461
pub type PyNativeFunc = FunctionBox<dyn Fn(&VirtualMachine, PyFuncArgs) -> PyResult + 'static>;
462+
// TODO: + Send + Sync
462463

463464
/// Implemented by types that are or can generate built-in functions.
464465
///

‎vm/src/obj/objmodule.rs

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ impl PyModuleRef {
5555
let zelf = PyModule {}.into_ref_with_type(vm, cls)?;
5656
init_module_dict(
5757
vm,
58-
&zelf.as_object().dict.as_ref().unwrap().borrow(),
58+
&zelf.as_object().dict().unwrap(),
5959
name.into_object(),
6060
doc.flat_option()
6161
.map_or_else(|| vm.get_none(), PyRef::into_object),
@@ -98,11 +98,8 @@ impl PyModuleRef {
9898
fn dir(self, vm: &VirtualMachine) -> PyResult<PyObjectRef> {
9999
let dict = self
100100
.as_object()
101-
.dict
102-
.as_ref()
103-
.ok_or_else(|| vm.new_value_error("module has no dict".to_owned()))?
104-
.borrow()
105-
.clone();
101+
.dict()
102+
.ok_or_else(|| vm.new_value_error("module has no dict".to_owned()))?;
106103
let attrs = dict.into_iter().map(|(k, _v)| k).collect();
107104
Ok(vm.ctx.new_list(attrs))
108105
}

‎vm/src/obj/objobject.rs

Lines changed: 13 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -108,8 +108,8 @@ impl PyBaseObject {
108108
}
109109
}
110110

111-
if let Some(ref dict) = obj.dict {
112-
dict.borrow().del_item(attr_name.as_str(), vm)?;
111+
if let Some(dict) = obj.dict() {
112+
dict.del_item(attr_name.as_str(), vm)?;
113113
Ok(())
114114
} else {
115115
Err(vm.new_attribute_error(format!(
@@ -142,10 +142,10 @@ impl PyBaseObject {
142142
let dict = PyDictRef::from_attributes(attributes, vm)?;
143143

144144
// Get instance attributes:
145-
if let Some(object_dict) = &obj.dict {
145+
if let Some(object_dict) = obj.dict() {
146146
vm.invoke(
147147
&vm.get_attribute(dict.clone().into_object(), "update")?,
148-
object_dict.borrow().clone().into_object(),
148+
object_dict.into_object(),
149149
)?;
150150
}
151151

@@ -187,24 +187,19 @@ impl PyBaseObject {
187187

188188
#[pyproperty(magic)]
189189
fn dict(object: PyObjectRef, vm: &VirtualMachine) -> PyResult<PyDictRef> {
190-
if let Some(ref dict) = object.dict {
191-
Ok(dict.borrow().clone())
192-
} else {
193-
Err(vm.new_attribute_error("no dictionary.".to_string()))
194-
}
190+
object
191+
.dict()
192+
.ok_or_else(|| vm.new_attribute_error("no dictionary.".to_owned()))
195193
}
196194

197195
#[pyproperty(magic, setter)]
198196
fn set_dict(instance: PyObjectRef, value: PyDictRef, vm: &VirtualMachine) -> PyResult<()> {
199-
if let Some(dict) = &instance.dict {
200-
*dict.borrow_mut() = value;
201-
Ok(())
202-
} else {
203-
Err(vm.new_attribute_error(format!(
197+
instance.set_dict(value).map_err(|_| {
198+
vm.new_attribute_error(format!(
204199
"'{}' object has no attribute '__dict__'",
205200
instance.class().name
206-
)))
207-
}
201+
))
202+
})
208203
}
209204

210205
#[pymethod(magic)]
@@ -248,8 +243,8 @@ pub(crate) fn setattr(
248243
}
249244
}
250245

251-
if let Some(ref dict) = obj.clone().dict {
252-
dict.borrow().set_item(attr_name.as_str(), value, vm)?;
246+
if let Some(dict) = obj.dict() {
247+
dict.set_item(attr_name.as_str(), value, vm)?;
253248
Ok(())
254249
} else {
255250
Err(vm.new_attribute_error(format!(

‎vm/src/pyobject.rs

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
11
use std::any::Any;
2-
use std::cell::{Cell, RefCell};
2+
use std::cell::Cell;
33
use std::collections::HashMap;
44
use std::fmt;
55
use std::marker::PhantomData;
66
use std::ops::Deref;
77
use std::rc::Rc;
8+
use std::sync::Mutex;
89

910
use indexmap::IndexMap;
1011
use num_bigint::BigInt;
@@ -572,7 +573,7 @@ impl PyContext {
572573
pub fn new_base_object(&self, class: PyClassRef, dict: Option<PyDictRef>) -> PyObjectRef {
573574
PyObject {
574575
typ: class.into_typed_pyobj(),
575-
dict: dict.map(RefCell::new),
576+
dict: dict.map(Mutex::new),
576577
payload: objobject::PyBaseObject,
577578
}
578579
.into_ref()
@@ -629,7 +630,8 @@ where
629630
T: ?Sized + PyObjectPayload,
630631
{
631632
pub typ: Rc<PyObject<PyClass>>,
632-
pub dict: Option<RefCell<PyDictRef>>, // __dict__ member
633+
// TODO: make this RwLock once PyObjectRef is Send + Sync
634+
pub(crate) dict: Option<Mutex<PyDictRef>>, // __dict__ member
633635
pub payload: T,
634636
}
635637

@@ -1131,7 +1133,7 @@ where
11311133
pub fn new(payload: T, typ: PyClassRef, dict: Option<PyDictRef>) -> PyObjectRef {
11321134
PyObject {
11331135
typ: typ.into_typed_pyobj(),
1134-
dict: dict.map(RefCell::new),
1136+
dict: dict.map(Mutex::new),
11351137
payload,
11361138
}
11371139
.into_ref()
@@ -1150,6 +1152,26 @@ where
11501152
}
11511153
}
11521154

1155+
impl<T> PyObject<T>
1156+
where
1157+
T: ?Sized + PyObjectPayload,
1158+
{
1159+
pub fn dict(&self) -> Option<PyDictRef> {
1160+
self.dict.as_ref().map(|mu| mu.lock().unwrap().clone())
1161+
}
1162+
/// Set the dict field. Returns `Err(dict)` if this object does not have a dict field
1163+
/// in the first place.
1164+
pub fn set_dict(&self, dict: PyDictRef) -> Result<(), PyDictRef> {
1165+
match self.dict {
1166+
Some(ref mu) => {
1167+
*mu.lock().unwrap() = dict;
1168+
Ok(())
1169+
}
1170+
None => Err(dict),
1171+
}
1172+
}
1173+
}
1174+
11531175
impl PyObject<dyn PyObjectPayload> {
11541176
#[inline]
11551177
pub fn payload<T: PyObjectPayload>(&self) -> Option<&T> {

‎vm/src/vm.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -967,8 +967,8 @@ impl VirtualMachine {
967967
}
968968
}
969969

970-
let attr = if let Some(ref dict) = obj.dict {
971-
dict.borrow().get_item_option(name_str.as_str(), self)?
970+
let attr = if let Some(dict) = obj.dict() {
971+
dict.get_item_option(name_str.as_str(), self)?
972972
} else {
973973
None
974974
};

0 commit comments

Comments
 (0)