diff --git a/Lib/test/test_marshal.py b/Lib/test/test_marshal.py index ad4c6095abf..1f04a7f697e 100644 --- a/Lib/test/test_marshal.py +++ b/Lib/test/test_marshal.py @@ -319,7 +319,6 @@ def test_recursion_limit(self): last.append([0]) self.assertRaises(ValueError, marshal.dumps, head) - @unittest.expectedFailure # TODO: RUSTPYTHON; ValueError: bad marshal data def test_reference_loop_list(self): a = [] a.append(a) @@ -331,7 +330,6 @@ def test_reference_loop_list(self): self.assertIsInstance(b, list) self.assertIs(b[0], b) - @unittest.expectedFailure # TODO: RUSTPYTHON; ValueError: bad marshal data def test_reference_loop_dict(self): a = {} a[None] = a @@ -343,7 +341,6 @@ def test_reference_loop_dict(self): self.assertIsInstance(b, dict) self.assertIs(b[None], b) - @unittest.expectedFailure # TODO: RUSTPYTHON; ValueError: bad marshal data def test_reference_loop_tuple(self): a = ([],) a[0].append(a) @@ -387,21 +384,18 @@ def test_reference_loop_slice(self): for v in range(marshal.version + 1): self.assertRaises(ValueError, marshal.dumps, a, v) - @unittest.expectedFailure # TODO: RUSTPYTHON; ValueError: bad marshal data def test_loads_reference_loop_list(self): data = b'\xdb\x01\x00\x00\x00r\x00\x00\x00\x00' # [] a = marshal.loads(data) self.assertIsInstance(a, list) self.assertIs(a[0], a) - @unittest.expectedFailure # TODO: RUSTPYTHON; ValueError: bad marshal data def test_loads_reference_loop_dict(self): data = b'\xfbNr\x00\x00\x00\x000' # {None: } a = marshal.loads(data) self.assertIsInstance(a, dict) self.assertIs(a[None], a) - @unittest.expectedFailure # TODO: RUSTPYTHON; ValueError: bad marshal data def test_loads_abnormal_reference_loops(self): # Indirect self-references of tuples. data = b'\xa8\x01\x00\x00\x00[\x01\x00\x00\x00r\x00\x00\x00\x00' # ([],) @@ -416,13 +410,13 @@ def test_loads_abnormal_reference_loops(self): self.assertIsInstance(a[0], dict) self.assertIs(a[0][None], a) - # Direct self-reference which cannot be created in Python. - # This creates a reference loop which cannot be collected. - if False: - data = b'\xa8\x01\x00\x00\x00r\x00\x00\x00\x00' # (,) - a = marshal.loads(data) - self.assertIsInstance(a, tuple) - self.assertIs(a[0], a) + # Direct self-reference which cannot be created in Python. CPython + # leaves this disabled because its reference counting cannot collect + # the resulting cycle; RustPython's tracing collector can. + data = b'\xa8\x01\x00\x00\x00r\x00\x00\x00\x00' # (,) + a = marshal.loads(data) + self.assertIsInstance(a, tuple) + self.assertIs(a[0], a) # Direct self-references which cannot be created in Python # because of unhashability. @@ -748,7 +742,6 @@ class InterningTestCase(unittest.TestCase, HelperMixin): strobj = "this is an interned string" strobj = sys.intern(strobj) - @unittest.expectedFailure # TODO: RUSTPYTHON def testIntern(self): s = marshal.loads(marshal.dumps(self.strobj)) self.assertEqual(s, self.strobj) diff --git a/crates/compiler-core/src/marshal.rs b/crates/compiler-core/src/marshal.rs index a2a23054e4b..dd5d4f2cddb 100644 --- a/crates/compiler-core/src/marshal.rs +++ b/crates/compiler-core/src/marshal.rs @@ -516,7 +516,7 @@ fn read_const_value( let code = deserialize_code_inner(rdr, bag, depth - 1, refs)?; bag.make_code(code) } else { - deserialize_value_typed(rdr, bag, depth, refs, typ)? + deserialize_value_typed(rdr, bag, depth, refs, typ, slot)? }; if let Some(idx) = slot { refs[idx] = Some(value.clone()); @@ -540,6 +540,10 @@ pub trait MarshalBag: Copy { fn make_str(&self, value: &Wtf8) -> Self::Value; + fn make_interned_str(&self, value: &Wtf8) -> Self::Value { + self.make_str(value) + } + fn make_bytes(&self, value: &[u8]) -> Self::Value; fn make_int(&self, value: BigInt) -> Self::Value; @@ -564,6 +568,51 @@ pub trait MarshalBag: Copy { it: impl Iterator, ) -> Result; + /// Install partially-built containers in the marshal reference table + /// before reading their children, as CPython's `r_object()` does. + /// Runtime bags can opt in; constant bags retain collect-then-construct. + fn make_tuple_placeholder(&self, _len: usize) -> Option { + None + } + + fn set_tuple_item( + &self, + _tuple: &Self::Value, + _index: usize, + _value: Self::Value, + ) -> Result<()> { + Err(MarshalError::BadType) + } + + fn make_list_placeholder(&self, _len: usize) -> Option { + None + } + + fn set_list_item(&self, _list: &Self::Value, _index: usize, _value: Self::Value) -> Result<()> { + Err(MarshalError::BadType) + } + + fn make_set_placeholder(&self) -> Option { + None + } + + fn insert_set_item(&self, _set: &Self::Value, _value: Self::Value) -> Result<()> { + Err(MarshalError::BadType) + } + + fn make_dict_placeholder(&self) -> Option { + None + } + + fn insert_dict_item( + &self, + _dict: &Self::Value, + _key: Self::Value, + _value: Self::Value, + ) -> Result<()> { + Err(MarshalError::BadType) + } + fn make_slice( &self, _start: Self::Value, @@ -755,7 +804,7 @@ fn deserialize_value_after_header( let code = deserialize_code_inner(rdr, bag.constant_bag(), depth - 1, &mut inner_refs)?; bag.make_code(code) } else { - deserialize_value_typed(rdr, bag, depth, refs, typ)? + deserialize_value_typed(rdr, bag, depth, refs, typ, slot)? }; if let Some(idx) = slot { @@ -770,6 +819,7 @@ fn deserialize_value_typed( depth: usize, refs: &mut Vec>, typ: Type, + slot: Option, ) -> Result { if depth == 0 { return Err(MarshalError::InvalidBytecode); @@ -806,21 +856,42 @@ fn deserialize_value_typed( let value = Complex64 { re, im }; bag.make_complex(value) } - Type::Ascii | Type::AsciiInterned | Type::Unicode | Type::Interned => { + Type::Ascii | Type::Unicode => { let len = rdr.read_u32()?; let value = rdr.read_wtf8(len)?; bag.make_str(value) } - Type::ShortAscii | Type::ShortAsciiInterned => { + Type::AsciiInterned | Type::Interned => { + let len = rdr.read_u32()?; + let value = rdr.read_wtf8(len)?; + bag.make_interned_str(value) + } + Type::ShortAscii => { let len = rdr.read_u8()? as u32; let value = rdr.read_wtf8(len)?; bag.make_str(value) } + Type::ShortAsciiInterned => { + let len = rdr.read_u8()? as u32; + let value = rdr.read_wtf8(len)?; + bag.make_interned_str(value) + } Type::SmallTuple => { let len = rdr.read_u8()? as usize; let d = depth - 1; - let it = (0..len).map(|_| deserialize_value_depth(rdr, bag, d, refs)); - itertools::process_results(it, |it| bag.make_tuple(it))? + if let Some(index) = slot + && let Some(tuple) = bag.make_tuple_placeholder(len) + { + refs[index] = Some(tuple.clone()); + for item_index in 0..len { + let item = deserialize_value_depth(rdr, bag, d, refs)?; + bag.set_tuple_item(&tuple, item_index, item)?; + } + tuple + } else { + let it = (0..len).map(|_| deserialize_value_depth(rdr, bag, d, refs)); + itertools::process_results(it, |it| bag.make_tuple(it))? + } } Type::Null => { return Err(MarshalError::BadType); @@ -830,22 +901,55 @@ fn deserialize_value_typed( return Err(MarshalError::BadType); } Type::Tuple => { - let len = rdr.read_u32()?; + let len = rdr.read_u32()? as usize; let d = depth - 1; - let it = (0..len).map(|_| deserialize_value_depth(rdr, bag, d, refs)); - itertools::process_results(it, |it| bag.make_tuple(it))? + if let Some(index) = slot + && let Some(tuple) = bag.make_tuple_placeholder(len) + { + refs[index] = Some(tuple.clone()); + for item_index in 0..len { + let item = deserialize_value_depth(rdr, bag, d, refs)?; + bag.set_tuple_item(&tuple, item_index, item)?; + } + tuple + } else { + let it = (0..len).map(|_| deserialize_value_depth(rdr, bag, d, refs)); + itertools::process_results(it, |it| bag.make_tuple(it))? + } } Type::List => { - let len = rdr.read_u32()?; + let len = rdr.read_u32()? as usize; let d = depth - 1; - let it = (0..len).map(|_| deserialize_value_depth(rdr, bag, d, refs)); - itertools::process_results(it, |it| bag.make_list(it))?? + if let Some(index) = slot + && let Some(list) = bag.make_list_placeholder(len) + { + refs[index] = Some(list.clone()); + for item_index in 0..len { + let item = deserialize_value_depth(rdr, bag, d, refs)?; + bag.set_list_item(&list, item_index, item)?; + } + list + } else { + let it = (0..len).map(|_| deserialize_value_depth(rdr, bag, d, refs)); + itertools::process_results(it, |it| bag.make_list(it))?? + } } Type::Set => { - let len = rdr.read_u32()?; + let len = rdr.read_u32()? as usize; let d = depth - 1; - let it = (0..len).map(|_| deserialize_value_depth(rdr, bag, d, refs)); - itertools::process_results(it, |it| bag.make_set(it))?? + if let Some(index) = slot + && let Some(set) = bag.make_set_placeholder() + { + refs[index] = Some(set.clone()); + for _ in 0..len { + let item = deserialize_value_depth(rdr, bag, d, refs)?; + bag.insert_set_item(&set, item)?; + } + set + } else { + let it = (0..len).map(|_| deserialize_value_depth(rdr, bag, d, refs)); + itertools::process_results(it, |it| bag.make_set(it))?? + } } Type::FrozenSet => { let len = rdr.read_u32()?; @@ -855,17 +959,33 @@ fn deserialize_value_typed( } Type::Dict => { let d = depth - 1; - let mut pairs = Vec::new(); - loop { - let raw = rdr.read_u8()?; - if raw & !FLAG_REF == b'0' { - break; + if let Some(index) = slot + && let Some(dict) = bag.make_dict_placeholder() + { + refs[index] = Some(dict.clone()); + loop { + let raw = rdr.read_u8()?; + if raw & !FLAG_REF == b'0' { + break; + } + let key = deserialize_value_after_header(rdr, bag, d, refs, raw)?; + let value = deserialize_value_depth(rdr, bag, d, refs)?; + bag.insert_dict_item(&dict, key, value)?; + } + dict + } else { + let mut pairs = Vec::new(); + loop { + let raw = rdr.read_u8()?; + if raw & !FLAG_REF == b'0' { + break; + } + let key = deserialize_value_after_header(rdr, bag, d, refs, raw)?; + let value = deserialize_value_depth(rdr, bag, d, refs)?; + pairs.push((key, value)); } - let k = deserialize_value_after_header(rdr, bag, d, refs, raw)?; - let v = deserialize_value_depth(rdr, bag, d, refs)?; - pairs.push((k, v)); + bag.make_dict(pairs.into_iter())? } - bag.make_dict(pairs.into_iter())? } Type::Bytes => { // After marshaling, byte arrays are converted into bytes. diff --git a/crates/vm/src/builtins/tuple.rs b/crates/vm/src/builtins/tuple.rs index d48639b2c11..06fa2519205 100644 --- a/crates/vm/src/builtins/tuple.rs +++ b/crates/vm/src/builtins/tuple.rs @@ -23,12 +23,60 @@ use crate::{ vm::VirtualMachine, }; use alloc::fmt; -use core::cell::Cell; +use core::cell::{Cell, UnsafeCell}; use core::ptr::NonNull; #[pyclass(module = false, name = "tuple", traverse = "manual")] pub struct PyTuple { - elements: Box<[R]>, + elements: TupleElements, +} + +/// Tuple storage is immutable after publication, but marshal must publish a +/// tuple in its reference table before recursively reading its children. +/// This mirrors CPython's `PyTuple_New` followed by `PyTuple_SET_ITEM`. +struct TupleElements(UnsafeCell>); + +unsafe impl Send for TupleElements {} +unsafe impl Sync for TupleElements {} + +impl TupleElements { + const fn new(elements: Box<[R]>) -> Self { + Self(UnsafeCell::new(elements)) + } + + fn as_slice(&self) -> &[R] { + // SAFETY: initialization writes happen only while the tuple is owned by + // the synchronous marshal decoder; afterwards the storage is immutable. + unsafe { &*self.0.get() } + } + + fn get_mut(&mut self) -> &mut Box<[R]> { + self.0.get_mut() + } + + /// # Safety + /// The tuple must still be in its private initialization phase, and each + /// placeholder index must be replaced at most once before it is observable. + unsafe fn set_initializing(&self, index: usize, value: R) { + unsafe { (*self.0.get())[index] = value }; + } +} + +impl core::ops::Deref for TupleElements { + type Target = [R]; + + fn deref(&self) -> &Self::Target { + self.as_slice() + } +} + +impl<'a, R> IntoIterator for &'a TupleElements { + type Item = &'a R; + type IntoIter = core::slice::Iter<'a, R>; + + fn into_iter(self) -> Self::IntoIter { + self.iter() + } } impl fmt::Debug for PyTuple { @@ -42,11 +90,11 @@ impl fmt::Debug for PyTuple { // Note: Only impl for PyTuple (the default) unsafe impl Traverse for PyTuple { fn traverse(&self, traverse_fn: &mut TraverseFn<'_>) { - self.elements.traverse(traverse_fn); + self.elements.as_slice().traverse(traverse_fn); } fn clear(&mut self, out: &mut Vec) { - let elements = core::mem::take(&mut self.elements); + let elements = core::mem::take(self.elements.get_mut()); out.extend(elements.into_vec()); } } @@ -206,7 +254,7 @@ impl Constructor for PyTuple { fn py_new(_cls: &Py, elements: Self::Args, _vm: &VirtualMachine) -> PyResult { Ok(Self { - elements: elements.into_boxed_slice(), + elements: TupleElements::new(elements.into_boxed_slice()), }) } } @@ -245,19 +293,19 @@ impl<'a, R> core::iter::IntoIterator for &'a Py> { impl PyTuple { #[must_use] - pub const fn as_slice(&self) -> &[R] { + pub fn as_slice(&self) -> &[R] { &self.elements } #[inline] #[must_use] - pub const fn len(&self) -> usize { + pub fn len(&self) -> usize { self.elements.len() } #[inline] #[must_use] - pub const fn is_empty(&self) -> bool { + pub fn is_empty(&self) -> bool { self.elements.is_empty() } @@ -274,7 +322,13 @@ impl PyTuple { ctx.empty_tuple.clone() } else { let elements = elements.into_boxed_slice(); - PyRef::new_ref(Self { elements }, ctx.types.tuple_type.to_owned(), None) + PyRef::new_ref( + Self { + elements: TupleElements::new(elements), + }, + ctx.types.tuple_type.to_owned(), + None, + ) } } @@ -283,7 +337,20 @@ impl PyTuple { /// Calling this function implies trying micro optimization for non-zero-sized tuple. #[must_use] pub const fn new_unchecked(elements: Box<[PyObjectRef]>) -> Self { - Self { elements } + Self { + elements: TupleElements::new(elements), + } + } + + pub(crate) fn new_marshal_placeholder(len: usize, ctx: &Context) -> PyRef { + Self::new_ref(vec![ctx.none(); len], ctx) + } + + /// # Safety + /// This tuple must be a marshal placeholder which has not escaped the + /// decoder, and `index` must not have been replaced previously. + pub(crate) unsafe fn set_marshal_item(&self, index: usize, value: PyObjectRef) { + unsafe { self.elements.set_initializing(index, value) }; } fn repeat(zelf: PyRef, value: isize, vm: &VirtualMachine) -> PyResult> { @@ -298,7 +365,10 @@ impl PyTuple { } else { let v = zelf.elements.mul(vm, value)?; let elements = v.into_boxed_slice(); - Self { elements }.into_ref(&vm.ctx) + Self { + elements: TupleElements::new(elements), + } + .into_ref(&vm.ctx) }) } @@ -341,7 +411,10 @@ impl PyTuple { .chain(other.as_slice()) .cloned() .collect::>(); - Self { elements }.into_ref(&vm.ctx) + Self { + elements: TupleElements::new(elements), + } + .into_ref(&vm.ctx) } }); PyArithmeticValue::from_option(added.ok()) @@ -360,7 +433,7 @@ impl PyTuple { #[inline] #[must_use] - pub const fn __len__(&self) -> usize { + pub fn __len__(&self) -> usize { self.elements.len() } @@ -425,7 +498,7 @@ impl PyTuple { let tup_arg = if zelf.class().is(vm.ctx.types.tuple_type) { zelf } else { - Self::new_ref(zelf.elements.clone().into_vec(), &vm.ctx) + Self::new_ref(zelf.elements.as_slice().to_vec(), &vm.ctx) }; (tup_arg,) } diff --git a/crates/vm/src/stdlib/marshal.rs b/crates/vm/src/stdlib/marshal.rs index ace3aff58f2..a4665d0f5b6 100644 --- a/crates/vm/src/stdlib/marshal.rs +++ b/crates/vm/src/stdlib/marshal.rs @@ -9,14 +9,16 @@ mod decl { use crate::{ PyObjectRef, PyResult, TryFromObject, VirtualMachine, builtins::{ - PyBool, PyByteArray, PyBytes, PyCode, PyComplex, PyDict, PyEllipsis, PyFloat, - PyFrozenSet, PyInt, PyList, PyNone, PySet, PyStopIteration, PyStr, PyTuple, + PyBaseExceptionRef, PyBool, PyByteArray, PyBytes, PyCode, PyComplex, PyDict, + PyEllipsis, PyFloat, PyFrozenSet, PyInt, PyList, PyNone, PySet, PyStopIteration, PyStr, + PyTuple, }, convert::ToPyObject, function::{ArgBytesLike, OptionalArg}, object::{AsObject, PyPayload}, protocol::PyBuffer, }; + use core::cell::RefCell; use malachite_bigint::BigInt; use num_traits::Zero; use rustpython_compiler_core::marshal::{self, DumpableValue}; @@ -386,79 +388,165 @@ mod decl { } #[derive(Copy, Clone)] - struct PyMarshalBag<'a>(&'a VirtualMachine); + struct PyMarshalBag<'a> { + vm: &'a VirtualMachine, + pending_error: &'a RefCell>, + } + + impl<'a> PyMarshalBag<'a> { + fn new( + vm: &'a VirtualMachine, + pending_error: &'a RefCell>, + ) -> Self { + Self { vm, pending_error } + } + + fn remember_python_error(&self, error: PyBaseExceptionRef) -> marshal::MarshalError { + let mut pending = self.pending_error.borrow_mut(); + if pending.is_none() { + *pending = Some(error); + } + marshal::MarshalError::BadType + } + } impl<'a> marshal::MarshalBag for PyMarshalBag<'a> { type Value = PyObjectRef; type ConstantBag = PyVmBag<'a>; fn make_bool(&self, value: bool) -> Self::Value { - self.0.ctx.new_bool(value).into() + self.vm.ctx.new_bool(value).into() } fn make_none(&self) -> Self::Value { - self.0.ctx.none() + self.vm.ctx.none() } fn make_ellipsis(&self) -> Self::Value { - self.0.ctx.ellipsis.clone().into() + self.vm.ctx.ellipsis.clone().into() } fn make_float(&self, value: f64) -> Self::Value { - self.0.ctx.new_float(value).into() + self.vm.ctx.new_float(value).into() } fn make_complex(&self, value: num_complex::Complex64) -> Self::Value { - self.0.ctx.new_complex(value).into() + self.vm.ctx.new_complex(value).into() } fn make_str(&self, value: &Wtf8) -> Self::Value { - self.0.ctx.new_str(value).into() + self.vm.ctx.new_str(value).into() + } + fn make_interned_str(&self, value: &Wtf8) -> Self::Value { + self.vm.ctx.intern_str(value).to_owned().into() } fn make_bytes(&self, value: &[u8]) -> Self::Value { - self.0.ctx.new_bytes(value.to_vec()).into() + self.vm.ctx.new_bytes(value.to_vec()).into() } fn make_int(&self, value: BigInt) -> Self::Value { - self.0.ctx.new_int(value).into() + self.vm.ctx.new_int(value).into() } fn make_tuple(&self, elements: impl Iterator) -> Self::Value { - self.0.ctx.new_tuple(elements.collect()).into() + self.vm.ctx.new_tuple(elements.collect()).into() + } + fn make_tuple_placeholder(&self, len: usize) -> Option { + Some(PyTuple::new_marshal_placeholder(len, &self.vm.ctx).into()) + } + fn set_tuple_item( + &self, + tuple: &Self::Value, + index: usize, + value: Self::Value, + ) -> Result<(), marshal::MarshalError> { + let tuple = tuple + .downcast_ref::() + .ok_or(marshal::MarshalError::BadType)?; + // SAFETY: compiler-core calls this only on a fresh placeholder, + // once per index, before returning it to Python code. + unsafe { tuple.set_marshal_item(index, value) }; + Ok(()) } fn make_code(&self, code: CodeObject) -> Self::Value { - crate::builtins::PyCode::new_ref_with_bag(self.0, code).into() + crate::builtins::PyCode::new_ref_with_bag(self.vm, code).into() } fn make_stop_iter(&self) -> Result { - Ok(self.0.ctx.exceptions.stop_iteration.to_owned().into()) + Ok(self.vm.ctx.exceptions.stop_iteration.to_owned().into()) } fn make_list( &self, it: impl Iterator, ) -> Result { - Ok(self.0.ctx.new_list(it.collect()).into()) + Ok(self.vm.ctx.new_list(it.collect()).into()) + } + fn make_list_placeholder(&self, len: usize) -> Option { + Some(self.vm.ctx.new_list(vec![self.vm.ctx.none(); len]).into()) + } + fn set_list_item( + &self, + list: &Self::Value, + index: usize, + value: Self::Value, + ) -> Result<(), marshal::MarshalError> { + let list = list + .downcast_ref::() + .ok_or(marshal::MarshalError::BadType)?; + list.borrow_vec_mut()[index] = value; + Ok(()) } fn make_set( &self, it: impl Iterator, ) -> Result { - let set = PySet::default().into_ref(&self.0.ctx); + let set = PySet::default().into_ref(&self.vm.ctx); for elem in it { - set.add(elem, self.0).unwrap() + set.add(elem, self.vm) + .map_err(|error| self.remember_python_error(error))?; } Ok(set.into()) } + fn make_set_placeholder(&self) -> Option { + Some(PySet::default().into_ref(&self.vm.ctx).into()) + } + fn insert_set_item( + &self, + set: &Self::Value, + value: Self::Value, + ) -> Result<(), marshal::MarshalError> { + let set = set + .downcast_ref::() + .ok_or(marshal::MarshalError::BadType)?; + set.add(value, self.vm) + .map_err(|error| self.remember_python_error(error)) + } fn make_frozenset( &self, it: impl Iterator, ) -> Result { - Ok(PyFrozenSet::from_iter(self.0, it) - .unwrap() - .to_pyobject(self.0)) + PyFrozenSet::from_iter(self.vm, it) + .map(|set| set.to_pyobject(self.vm)) + .map_err(|error| self.remember_python_error(error)) } fn make_dict( &self, it: impl Iterator, ) -> Result { - let dict = self.0.ctx.new_dict(); + let dict = self.vm.ctx.new_dict(); for (k, v) in it { - dict.set_item(&*k, v, self.0).unwrap() + dict.set_item(&*k, v, self.vm) + .map_err(|error| self.remember_python_error(error))?; } Ok(dict.into()) } + fn make_dict_placeholder(&self) -> Option { + Some(self.vm.ctx.new_dict().into()) + } + fn insert_dict_item( + &self, + dict: &Self::Value, + key: Self::Value, + value: Self::Value, + ) -> Result<(), marshal::MarshalError> { + let dict = dict + .downcast_ref::() + .ok_or(marshal::MarshalError::BadType)?; + dict.set_item(&*key, value, self.vm) + .map_err(|error| self.remember_python_error(error)) + } fn make_slice( &self, start: Self::Value, @@ -466,7 +554,7 @@ mod decl { step: Self::Value, ) -> Result { use crate::builtins::PySlice; - let vm = self.0; + let vm = self.vm; Ok(PySlice { start: if vm.is_none(&start) { None @@ -480,7 +568,21 @@ mod decl { .into()) } fn constant_bag(self) -> Self::ConstantBag { - PyVmBag(self.0) + PyVmBag(self.vm) + } + } + + fn deserialize_value( + rdr: &mut impl marshal::Read, + vm: &VirtualMachine, + ) -> PyResult { + let pending_error = RefCell::new(None); + match marshal::deserialize_value(rdr, PyMarshalBag::new(vm, &pending_error)) { + Ok(value) => Ok(value), + Err(error) => Err(pending_error.into_inner().unwrap_or_else(|| match error { + marshal::MarshalError::Eof => vm.new_eof_error("marshal data too short"), + _ => vm.new_value_error("bad marshal data"), + })), } } @@ -502,11 +604,7 @@ mod decl { vm.new_buffer_error("Buffer provided to marshal.loads() is not contiguous") })?; - let result = - marshal::deserialize_value(&mut &buf[..], PyMarshalBag(vm)).map_err(|e| match e { - marshal::MarshalError::Eof => vm.new_eof_error("marshal data too short"), - _ => vm.new_value_error("bad marshal data"), - })?; + let result = deserialize_value(&mut &buf[..], vm)?; if !allow_code { check_no_code(&result, vm)?; } @@ -534,14 +632,7 @@ mod decl { let mut rdr: &[u8] = &buf; let len_before = rdr.len(); - let result = - marshal::deserialize_value(&mut rdr, PyMarshalBag(vm)).map_err(|e| match e { - marshal::MarshalError::Eof => vm.new_exception_msg( - vm.ctx.exceptions.eof_error.to_owned(), - "marshal data too short".into(), - ), - _ => vm.new_value_error("bad marshal data"), - })?; + let result = deserialize_value(&mut rdr, vm)?; let consumed = len_before - rdr.len(); // Seek file to just after the consumed bytes