GeezerVison is showing github.com — Support owned exception payload construction by JMLX42 · Pull Request #9734 · RustPython/RustPython
Support owned exception payload construction
#9734
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and
privacy statement . We’ll occasionally send you account related emails.
Already on GitHub?
Sign in
to your account
Uh oh!
There was an error while loading. Please reload this page.
Show file tree
Hide file tree
Changes from all commits
Commits
File filter
Filter by extension
.rs
(3)
All 1 file type selected
Viewed files
Conversations
Failed to load comments.
Loading
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Jump to file
Failed to load files.
Loading
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Hide whitespace
Diff view
Hide whitespace
There are no files selected for viewing
Original file line number
Diff line number
Diff line change
Expand Up
@@ -1003,9 +1003,59 @@ pub(crate) fn impl_pyexception(attr: PunctuatedNestedMeta, item: &Item) -> Resul
// Forward a `traverse` option to the generated `#[pyclass]` so exception
// payloads with a manual `Traverse` impl are GC-tracked and traversed.
let traverse_attr = match class_meta.inner()._optional_str("traverse").ok().flatten() {
Some(value) => quote! { , traverse = #value },
None => quote! {},
// Owned one-field wrappers must expose the base's references when allocated
// with their own vtable, rather than through the base's allocation slot.
let is_wrapper = matches!(item, Item::Struct(item) if matches!(&item.fields,
syn::Fields::Unnamed(fields) if fields.unnamed.len() == 1));
// One-field wrappers implement owned construction. Other shapes keep the
// trait default, which leaves allocation on slot_new.
let owned_constructor_impl = if is_wrapper {
quote! {
impl ::rustpython_vm::exceptions::PyExceptionOwnedConstructor for #ident {
/// Wraps the owned payload returned by the base constructor.
fn __pyexception_new<F>(
new_base: F,
) -> ::rustpython_vm::PyResult<Self>
where
F: ::core::ops::FnOnce() -> ::rustpython_vm::PyResult<
<Self as ::rustpython_vm::class::PyClassDef>::Base,
>,
{
Ok(Self(new_base()?))
}
}
}
} else {
quote! {
impl ::rustpython_vm::exceptions::PyExceptionOwnedConstructor for #ident {}
}
};
let (traverse_attr, traverse_impl) = match class_meta
.inner()
._optional_str("traverse")
.ok()
.flatten()
{
Some(value) => (quote! { , traverse = #value }, quote! {}),
None if is_wrapper => (
quote! { , traverse = "manual" },
quote! {
// SAFETY: The sole field owns all payload references. Forward
// to its traversal once without cloning any references.
unsafe impl ::rustpython_vm::object::Traverse for #ident {
/// Visits the sole field's Python references without cloning them.
fn traverse(&self, tracer_fn: &mut ::rustpython_vm::object::TraverseFn<'_>) {
::rustpython_vm::object::MaybeTraverse::try_traverse(&self.0, tracer_fn);
}
/// Forwards cycle breaking to the sole field's clear implementation.
fn clear(&mut self, out: &mut ::std::vec::Vec<::rustpython_vm::PyObjectRef>) {
::rustpython_vm::object::MaybeTraverse::try_clear(&mut self.0, out);
}
}
},
),
None => (quote! {}, quote! {}),
Comment thread
JMLX42 marked this conversation as resolved.
Resolved
Uh oh!
There was an error while loading. Please reload this page.
};
let payload_attr = match class_meta.inner()._optional_str("payload")? {
Expand All
@@ -1021,11 +1071,17 @@ pub(crate) fn impl_pyexception(attr: PunctuatedNestedMeta, item: &Item) -> Resul
let ret = quote! {
#[pyclass(#module_attr name = #class_name, base = #base_class_name #traverse_attr #payload_attr)]
#item
#owned_constructor_impl
#traverse_impl
#impl_pyclass
};
Ok(ret)
}
/// Expands exception methods while preserving custom constructors and base allocation slots.
///
/// Unless `with(Constructor)` supplies a custom implementation, owned construction
/// uses the struct-generated helper and Python allocation delegates to the base.
pub(crate) fn impl_pyexception_impl(attr: PunctuatedNestedMeta, item: Item) -> TokenStream {
let Item::Impl(imp) = item else {
return item.into_token_stream();
Expand Down
Expand Up
@@ -1079,12 +1135,15 @@ pub(crate) fn impl_pyexception_impl(attr: PunctuatedNestedMeta, item: Item) -> T
) -> ::rustpython_vm::PyResult {
<Self as ::rustpython_vm::class::PyClassDef>::Base::slot_new(cls, args, vm)
}
/// Constructs an owned wrapper through its base payload constructor.
fn py_new(
_cls : &::rustpython_vm::Py<::rustpython_vm::builtins::PyType>,
_args : Self::Args,
_vm : &::rustpython_vm::VirtualMachine
cls : &::rustpython_vm::Py<::rustpython_vm::builtins::PyType>,
args : Self::Args,
vm : &::rustpython_vm::VirtualMachine
) -> ::rustpython_vm::PyResult<Self> {
unreachable!("slot_new is defined")
<Self as ::rustpython_vm::exceptions::PyExceptionOwnedConstructor>::__pyexception_new(|| {
<<Self as ::rustpython_vm::class::PyClassDef>::Base as ::rustpython_vm::types::Constructor>::py_new(cls, args, vm)
})
}
}
}
Expand Down
Original file line number
Diff line number
Diff line change
Expand Up
@@ -32,6 +32,25 @@ unsafe impl Traverse for PyBaseException {
self.context.traverse(tracer_fn);
self.args.traverse(tracer_fn);
}
/// Detaches traceback, cause, and context references for deferred release.
///
/// Moving references into `out` breaks these cycles without dropping children
/// during the collector's exclusive payload access. Arguments remain owned by
/// the payload until its destruction.
fn clear(&mut self, out: &mut Vec<PyObjectRef>) {
if let Some(traceback) = self.traceback.get_mut().take() {
out.push(traceback.into());
}
if let Some(cause) = self.cause.get_mut().take() {
out.push(cause.into());
}
if let Some(context) = self.context.get_mut().take() {
out.push(context.into());
}
}
}
impl core::fmt::Debug for PyBaseException {
Expand Down
Expand Up
@@ -1041,8 +1060,9 @@ impl Constructor for PyBaseException {
.map(Into::into)
}
fn py_new(_cls: &Py<PyType>, _args: FuncArgs, _vm: &VirtualMachine) -> PyResult<Self> {
unimplemented!("use slot_new")
/// Creates an unallocated base payload preserving the supplied positional arguments.
fn py_new(_cls: &Py<PyType>, args: FuncArgs, vm: &VirtualMachine) -> PyResult<Self> {
Ok(Self::new(args.args, vm))
}
}
Expand Down
Expand Up
@@ -1879,6 +1899,29 @@ impl ToPyException for rustpython_host_env::multiprocessing::SemError {
}
}
// Generated `Constructor::py_new` calls this. One-field wrappers override it
// with owned construction; other shapes keep the default and allocate via
// `slot_new`.
#[doc(hidden)]
/// Connects generated exception constructors to shape-specific owned construction.
pub trait PyExceptionOwnedConstructor: crate::class::PyClassDef + Sized {
/// Rejects owned construction unless the wrapper supplies an implementation.
///
/// The default does not invoke `new_base`. Generated single-field wrappers
/// override it to wrap the base payload and propagate constructor errors.
///
/// # Panics
///
/// Panics when this default is called for a shape requiring explicit construction.
fn __pyexception_new<F>(new_base: F) -> crate::PyResult<Self>
where
F: FnOnce() -> crate::PyResult<Self::Base>,
{
let _ = new_base;
unreachable!("slot_new is defined")
}
}
pub(super) mod types {
use crate::class::PyClassDef;
use crate::common::lock::PyRwLock;
Expand Down
Expand Up
@@ -2455,19 +2498,9 @@ pub(super) mod types {
self.0.0.traverse(tracer_fn);
}
/// Delegates cycle breaking to the shared base-exception payload.
fn clear(&mut self, out: &mut Vec<PyObjectRef>) {
let base = &mut self.0.0;
if let Some(traceback) = base.traceback.get_mut().take() {
out.push(traceback.into());
}
if let Some(cause) = base.cause.get_mut().take() {
out.push(cause.into());
}
if let Some(context) = base.context.get_mut().take() {
out.push(context.into());
}
self.0.0.clear(out);
}
}
Expand Down
Expand Up
@@ -2589,11 +2622,9 @@ pub(super) mod types {
}
unsafe impl Traverse for PyOSError {
/// Visits base-exception references and the operating-system error's object fields.
fn traverse(&self, tracer_fn: &mut TraverseFn<'_>) {
// `self.base` is a `PyException` newtype whose `MaybeTraverse` is a
// no-op; reach the underlying `PyBaseException` so its traceback,
// cause, context and args are visited by the collector.
self.base.0.traverse(tracer_fn);
self.base.traverse(tracer_fn);
if let Some(obj) = self.errno.deref() {
tracer_fn(obj);
}
Expand Down
Original file line number
Diff line number
Diff line change
@@ -0,0 +1,138 @@
#![cfg(test)]
#![cfg(all(feature = "compiler", feature = "gc"))]
extern crate alloc;
use alloc::sync::Arc;
use core::sync::atomic::{AtomicBool, Ordering};
use rustpython_vm::{
AsObject, Interpreter, Py, PyObjectRef, PyPayload, PyResult, VirtualMachine,
builtins::{PyBaseException, PyType, PyValueError},
class::{PyClassImpl, PySubclass},
function::FuncArgs,
object::{Traverse, TraverseFn},
pyclass,
types::Constructor,
};
static SERIAL: std::sync::Mutex<()> = std::sync::Mutex::new(());
#[pyclass(module = "exception_payload_test", name = "DropError", base = PyValueError, traverse = "manual")]
#[repr(C)]
#[derive(Debug)]
struct DropError {
base: PyValueError,
dropped: Arc<AtomicBool>,
}
// SAFETY: repr(C) keeps the valid base first; pyclass checks offset and alignment.
unsafe impl PySubclass for DropError {
type Base = PyValueError;
fn as_base(&self) -> &Self::Base {
&self.base
}
}
// SAFETY: The base owns every Python reference; the flag owns none.
unsafe impl Traverse for DropError {
fn traverse(&self, tracer_fn: &mut TraverseFn<'_>) {
self.base.traverse(tracer_fn);
}
fn clear(&mut self, out: &mut Vec<PyObjectRef>) {
self.base.clear(out);
}
}
#[pyclass(with(Constructor))]
impl DropError {}
impl Constructor for DropError {
type Args = FuncArgs;
fn py_new(cls: &Py<PyType>, args: FuncArgs, vm: &VirtualMachine) -> PyResult<Self> {
Ok(Self {
base: PyValueError::py_new(cls, args, vm)?,
dropped: Arc::new(AtomicBool::new(false)),
})
}
}
impl Drop for DropError {
fn drop(&mut self) {
self.dropped.store(true, Ordering::SeqCst);
}
}
#[test]
fn owned_native_exception_round_trips_through_python() {
let _serial = SERIAL.lock().unwrap_or_else(|e| e.into_inner());
Interpreter::without_stdlib(Default::default()).enter(|vm| {
let class = DropError::make_static_type();
let argument = vm.ctx.new_str("payload");
let payload = DropError::py_new(&class, vec![argument.clone().into()].into(), vm).unwrap();
let dropped = payload.dropped.clone();
let exception = payload.into_ref_with_type(vm, class).unwrap();
let base = exception.as_base().as_base().as_base();
assert!(base.__traceback__().is_none());
assert!(base.__cause__().is_none());
assert!(base.__context__().is_none());
assert!(
!exception
.as_object()
.get_attr("__suppress_context__", vm)
.unwrap()
.try_to_bool(vm)
.unwrap()
);
assert!(!dropped.load(Ordering::SeqCst));
let scope = vm.new_scope_with_builtins();
scope.globals.set_item("exc", exception.into(), vm).unwrap();
scope.globals.set_item("arg", argument.into(), vm).unwrap();
let source = r#"
try:
raise exc
except ValueError as e:
assert e.args == (arg,)
assert isinstance(e, ValueError)
caught = e
"#;
let code = vm
.compile(
source,
rustpython_vm::compiler::Mode::Exec,
"<native-exception>",
)
.map_err(|error| error.into_pyexception(vm, Some(source)))
.unwrap();
if let Err(error) = vm.run_code_obj(code, scope.clone()) {
vm.print_exception(&error);
panic!("owned native exception round-trip failed");
}
let caught = scope.globals.get_item("caught", vm).unwrap();
let native = caught.downcast::<DropError>().unwrap();
assert!(Arc::ptr_eq(&native.dropped, &dropped));
assert!(!native.dropped.load(Ordering::SeqCst));
});
}
#[test]
fn context_cycle_destroys_owned_exception_payload() {
let _serial = SERIAL.lock().unwrap_or_else(|e| e.into_inner());
Interpreter::without_stdlib(Default::default()).enter(|vm| {
let class = DropError::make_static_type();
let payload = DropError::py_new(&class, FuncArgs::default(), vm).unwrap();
let dropped = payload.dropped.clone();
let exception = payload.into_ref_with_type(vm, class).unwrap();
let base = exception.as_base().as_base().as_base();
base.set_context(Some(exception.clone().upcast::<PyBaseException>()));
assert!(base.__context__().unwrap().is(exception.as_object()));
assert!(!dropped.load(Ordering::SeqCst));
drop(exception);
assert!(!dropped.load(Ordering::SeqCst));
vm.state.gc.collect_force(2);
assert!(dropped.load(Ordering::SeqCst));
});
}
Loading
Oops, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.