Skip to content

Commit a2fe158

Browse files
youknowoneyouknowone
committed
Fix ext detection, HeapMethodDef ownership, WASM error
- Remove HAS_EXT gc_bits flag; detect ext from type flags using raw pointer reads to avoid Stacked Borrows violations - Store HeapMethodDef owner in payload instead of dict hack - Clear dict entries in gc_clear_raw to break cycles - Add WASM error fallback when exception serialization fails
1 parent fb20298 commit a2fe158

File tree

7 files changed

+61
-27
lines changed

7 files changed

+61
-27
lines changed

‎.cspell.dict/rust-more.txt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ bindgen
66
bitand
77
bitflags
88
bitor
9+
bitvec
910
bitxor
1011
bstr
1112
byteorder
@@ -58,6 +59,7 @@ powi
5859
prepended
5960
punct
6061
replacen
62+
retag
6163
rmatch
6264
rposition
6365
rsplitn
@@ -89,5 +91,3 @@ widestring
8991
winapi
9092
winresource
9193
winsock
92-
bitvec
93-
Bitvec

‎crates/vm/src/builtins/builtin_func.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@ pub struct PyNativeFunction {
1616
pub(crate) value: &'static PyMethodDef,
1717
pub(crate) zelf: Option<PyObjectRef>,
1818
pub(crate) module: Option<&'static PyStrInterned>, // None for bound method
19+
/// Prevent HeapMethodDef from being freed while this function references it
20+
pub(crate) _method_def_owner: Option<PyObjectRef>,
1921
}
2022

2123
impl PyPayload for PyNativeFunction {

‎crates/vm/src/builtins/descriptor.rs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,8 @@ pub struct PyMethodDescriptor {
3737
pub method: &'static PyMethodDef,
3838
// vectorcall: vector_call_func,
3939
pub objclass: &'static Py<PyType>, // TODO: move to tp_members
40+
/// Prevent HeapMethodDef from being freed while this descriptor references it
41+
pub(crate) _method_def_owner: Option<PyObjectRef>,
4042
}
4143

4244
impl PyMethodDescriptor {
@@ -49,6 +51,7 @@ impl PyMethodDescriptor {
4951
},
5052
method,
5153
objclass: typ,
54+
_method_def_owner: None,
5255
}
5356
}
5457
}

‎crates/vm/src/function/method.rs‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,7 @@ impl PyMethodDef {
123123
zelf: None,
124124
value: self,
125125
module: None,
126+
_method_def_owner: None,
126127
}
127128
}
128129

@@ -144,6 +145,7 @@ impl PyMethodDef {
144145
zelf: Some(obj),
145146
value: self,
146147
module: None,
148+
_method_def_owner: None,
147149
},
148150
class,
149151
}
@@ -162,6 +164,7 @@ impl PyMethodDef {
162164
zelf: Some(obj),
163165
value: self,
164166
module: None,
167+
_method_def_owner: None,
165168
};
166169
PyRef::new_ref(
167170
function,
@@ -217,6 +220,7 @@ impl PyMethodDef {
217220
zelf: Some(class.to_owned().into()),
218221
value: self,
219222
module: None,
223+
_method_def_owner: None,
220224
};
221225
PyNativeMethod { func, class }.into_ref(ctx)
222226
}
@@ -293,14 +297,12 @@ impl Py<HeapMethodDef> {
293297
}
294298

295299
pub fn build_function(&self, vm: &VirtualMachine) -> PyRef<PyNativeFunction> {
296-
let function = unsafe { self.method() }.to_function();
297-
let dict = vm.ctx.new_dict();
298-
dict.set_item("__method_def__", self.to_owned().into(), vm)
299-
.unwrap();
300+
let mut function = unsafe { self.method() }.to_function();
301+
function._method_def_owner = Some(self.to_owned().into());
300302
PyRef::new_ref(
301303
function,
302304
vm.ctx.types.builtin_function_or_method_type.to_owned(),
303-
Some(dict),
305+
None,
304306
)
305307
}
306308

@@ -309,14 +311,12 @@ impl Py<HeapMethodDef> {
309311
class: &'static Py<PyType>,
310312
vm: &VirtualMachine,
311313
) -> PyRef<PyMethodDescriptor> {
312-
let function = unsafe { self.method() }.to_method(class, &vm.ctx);
313-
let dict = vm.ctx.new_dict();
314-
dict.set_item("__method_def__", self.to_owned().into(), vm)
315-
.unwrap();
314+
let mut function = unsafe { self.method() }.to_method(class, &vm.ctx);
315+
function._method_def_owner = Some(self.to_owned().into());
316316
PyRef::new_ref(
317317
function,
318318
vm.ctx.types.method_descriptor_type.to_owned(),
319-
Some(dict),
319+
None,
320320
)
321321
}
322322
}

‎crates/vm/src/object/core.rs‎

Lines changed: 27 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ use super::{
1717
};
1818
use crate::object::traverse_object::PyObjVTable;
1919
use crate::{
20-
builtins::{PyDictRef, PyType, PyTypeRef},
20+
builtins::{PyDict, PyDictRef, PyType, PyTypeRef},
2121
common::{
2222
atomic::{Ordering, PyAtomic, Radium},
2323
linked_list::{Link, Pointers},
@@ -257,8 +257,6 @@ bitflags::bitflags! {
257257
const SHARED_INLINE = 1 << 5;
258258
/// Use deferred reference counting
259259
const DEFERRED = 1 << 6;
260-
/// Object has ObjExt prefix allocation
261-
const HAS_EXT = 1 << 7;
262260
}
263261
}
264262

@@ -356,11 +354,20 @@ pub(super) struct PyInner<T> {
356354
pub(crate) const SIZEOF_PYOBJECT_HEAD: usize = core::mem::size_of::<PyInner<()>>();
357355

358356
impl<T> PyInner<T> {
359-
/// Check if this object has an ObjExt prefix.
360-
/// Uses the per-instance HAS_EXT bit in gc_bits, set at allocation time.
357+
/// Check if this object has an ObjExt prefix based on type flags.
358+
/// Uses raw pointer reads to avoid Stacked Borrows violations during bootstrap,
359+
/// where type objects have self-referential typ pointers that may be mutated.
361360
#[inline(always)]
362361
fn has_ext(&self) -> bool {
363-
GcBits::from_bits_retain(self.gc_bits.load(Ordering::Relaxed)).contains(GcBits::HAS_EXT)
362+
// Read slots via raw pointers only — creating a &Py<PyType> reference
363+
// would retag the entire object, conflicting with &mut writes during bootstrap.
364+
let typ_ptr = self.typ.load_raw();
365+
let slots = unsafe { core::ptr::addr_of!((*typ_ptr).0.payload.slots) };
366+
let flags = unsafe { core::ptr::addr_of!((*slots).flags).read() };
367+
let member_count = unsafe { core::ptr::addr_of!((*slots).member_count).read() };
368+
flags.has_feature(crate::types::PyTypeFlags::HAS_DICT)
369+
|| flags.has_feature(crate::types::PyTypeFlags::HAS_WEAKREF)
370+
|| member_count > 0
364371
}
365372

366373
/// Access the ObjExt prefix at a negative offset from this PyInner.
@@ -943,16 +950,20 @@ impl<T: PyPayload + core::fmt::Debug> PyInner<T> {
943950
/// For objects with ext, the allocation layout is: [ObjExt][PyInner<T>]
944951
fn new(payload: T, typ: PyTypeRef, dict: Option<PyDictRef>) -> *mut Self {
945952
let member_count = typ.slots.member_count;
946-
let needs_ext = dict.is_some()
947-
|| typ
948-
.slots
949-
.flags
950-
.has_feature(crate::types::PyTypeFlags::HAS_DICT)
953+
let needs_ext = typ
954+
.slots
955+
.flags
956+
.has_feature(crate::types::PyTypeFlags::HAS_DICT)
951957
|| typ
952958
.slots
953959
.flags
954960
.has_feature(crate::types::PyTypeFlags::HAS_WEAKREF)
955961
|| member_count > 0;
962+
debug_assert!(
963+
needs_ext || dict.is_none(),
964+
"dict passed to type '{}' without HAS_DICT flag",
965+
typ.name()
966+
);
956967

957968
if needs_ext {
958969
let ext_layout = core::alloc::Layout::new::<ObjExt>();
@@ -975,7 +986,7 @@ impl<T: PyPayload + core::fmt::Debug> PyInner<T> {
975986
inner_ptr.write(Self {
976987
ref_count: RefCount::new(),
977988
vtable: PyObjVTable::of::<T>(),
978-
gc_bits: Radium::new(GcBits::HAS_EXT.bits()),
989+
gc_bits: Radium::new(0),
979990
gc_generation: Radium::new(GC_UNTRACKED),
980991
gc_pointers: Pointers::new(),
981992
typ: PyAtomicRef::from(typ),
@@ -1660,6 +1671,8 @@ impl PyObject {
16601671
if let Some(ext) = obj.0.ext_ref() {
16611672
if let Some(dict) = ext.dict.as_ref() {
16621673
let dict_ref = dict.get();
1674+
// Clear dict entries to break cycles, then collect the dict itself
1675+
PyDict::clear(&dict_ref);
16631676
result.push(dict_ref.into());
16641677
}
16651678
for slot in ext.slots.iter() {
@@ -2330,7 +2343,7 @@ pub(crate) fn init_type_hierarchy() -> (PyTypeRef, PyTypeRef, PyTypeRef) {
23302343
PyInner::<PyType> {
23312344
ref_count: RefCount::new(),
23322345
vtable: PyObjVTable::of::<PyType>(),
2333-
gc_bits: Radium::new(GcBits::HAS_EXT.bits()),
2346+
gc_bits: Radium::new(0),
23342347
gc_generation: Radium::new(GC_UNTRACKED),
23352348
gc_pointers: Pointers::new(),
23362349
payload: type_payload,
@@ -2345,7 +2358,7 @@ pub(crate) fn init_type_hierarchy() -> (PyTypeRef, PyTypeRef, PyTypeRef) {
23452358
PyInner::<PyType> {
23462359
ref_count: RefCount::new(),
23472360
vtable: PyObjVTable::of::<PyType>(),
2348-
gc_bits: Radium::new(GcBits::HAS_EXT.bits()),
2361+
gc_bits: Radium::new(0),
23492362
gc_generation: Radium::new(GC_UNTRACKED),
23502363
gc_pointers: Pointers::new(),
23512364
payload: object_payload,

‎crates/vm/src/object/ext.rs‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -315,6 +315,14 @@ impl<T: PyPayload> Deref for PyAtomicRef<T> {
315315
}
316316

317317
impl<T: PyPayload> PyAtomicRef<T> {
318+
/// Load the raw pointer without creating a reference.
319+
/// Avoids Stacked Borrows retag, safe for use during bootstrap
320+
/// when type objects have self-referential pointers being mutated.
321+
#[inline(always)]
322+
pub(super) fn load_raw(&self) -> *const Py<T> {
323+
self.inner.load(Ordering::Relaxed).cast::<Py<T>>()
324+
}
325+
318326
/// # Safety
319327
/// The caller is responsible to keep the returned PyRef alive
320328
/// until no more reference can be used via PyAtomicRef::deref()

‎crates/wasm/src/convert.rs‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,15 @@ pub fn py_err_to_js_err(vm: &VirtualMachine, py_err: &Py<PyBaseException>) -> Js
4949
serde_wasm_bindgen::to_value(&exceptions::SerializeException::new(vm, py_err));
5050
match res {
5151
Ok(err_info) => PyError::new(err_info).into(),
52-
Err(e) => e.into(),
52+
Err(_) => {
53+
// Fallback: create a basic JS Error with the exception type and message
54+
let exc_type = py_err.class().name().to_string();
55+
let msg = match py_err.as_object().str(vm) {
56+
Ok(s) => format!("{exc_type}: {s}"),
57+
Err(_) => exc_type,
58+
};
59+
js_sys::Error::new(&msg).into()
60+
}
5361
}
5462
}
5563
}

0 commit comments

Comments
 (0)