Skip to content

Commit b47d20f

Browse files
youknowdotcodex
andauthored
Avoid the result-vector allocation for a single ctypes output (#9749)
Keep OutValue and all owners alive across output conversion. Exercise the real VM/libffi return path with a native test fixture, without enabling the separately incomplete DLL paramflags constructor. Assisted-by: OpenAI Codex:model version not exposed Co-authored-by: Codex <codex@openai.com>
1 parent 5713e58 commit b47d20f

1 file changed

Lines changed: 146 additions & 14 deletions

File tree

‎crates/vm/src/stdlib/_ctypes/function.rs‎

Lines changed: 146 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1795,15 +1795,12 @@ fn pycdata_from_ffi_result(
17951795
Ok(instance)
17961796
}
17971797

1798-
/// Extract values from OUT buffers
1799-
fn extract_out_values(out_buffers: &[OutValue], vm: &VirtualMachine) -> PyResult<Vec<PyObjectRef>> {
1800-
out_buffers
1801-
.iter()
1802-
.map(|value| match value {
1803-
OutValue::Out(buffer) => vm.call_method(buffer, "__ctypes_from_outparam__", ()),
1804-
OutValue::InOut(buffer) => Ok(buffer.clone()),
1805-
})
1806-
.collect()
1798+
/// Extract one OUT parameter without consuming its owner.
1799+
fn extract_out_value(value: &OutValue, vm: &VirtualMachine) -> PyResult {
1800+
match value {
1801+
OutValue::Out(buffer) => vm.call_method(buffer, "__ctypes_from_outparam__", ()),
1802+
OutValue::InOut(buffer) => Ok(buffer.clone()),
1803+
}
18071804
}
18081805

18091806
/// Build final result (main function)
@@ -1871,11 +1868,14 @@ fn build_result(
18711868
// OUT parameters replace the raw result. Release it before conversion,
18721869
// which can invoke user-defined methods.
18731870
drop(result);
1874-
let out_values = extract_out_values(&out_buffers, vm)?;
1875-
Ok(match <[PyObjectRef; 1]>::try_from(out_values) {
1876-
Ok([single]) => single,
1877-
Err(v) => PyTuple::new_ref(v, &vm.ctx).into(),
1878-
})
1871+
if let [single] = out_buffers.as_slice() {
1872+
return extract_out_value(single, vm);
1873+
}
1874+
let out_values = out_buffers
1875+
.iter()
1876+
.map(|value| extract_out_value(value, vm))
1877+
.collect::<PyResult<Vec<_>>>()?;
1878+
Ok(PyTuple::new_ref(out_values, &vm.ctx).into())
18791879
}
18801880

18811881
impl Callable for PyCFuncPtr {
@@ -2491,6 +2491,138 @@ mod tests {
24912491
use super::*;
24922492
use crate::{TryFromObject, class::PyClassImpl, function::ArgBytesLike};
24932493

2494+
#[cfg(all(feature = "compiler", not(target_family = "wasm")))]
2495+
#[test]
2496+
fn native_out_returns_preserve_python_protocol() {
2497+
extern "C" fn fill_one(output: *mut i32) -> i32 {
2498+
// The ctypes pointer argument owns an initialized C-int buffer.
2499+
unsafe { *output = 11 };
2500+
33
2501+
}
2502+
extern "C" fn fill_two(first: *mut i32, second: *mut i32) -> i32 {
2503+
// Both arguments are backed by live ctypes C-int buffers.
2504+
unsafe {
2505+
*first = 11;
2506+
*second = 22;
2507+
}
2508+
33
2509+
}
2510+
crate::Interpreter::without_stdlib(Default::default()).enter(|vm| {
2511+
let scope = vm.new_scope_with_builtins();
2512+
scope
2513+
.globals
2514+
.set_item(
2515+
"address_one",
2516+
vm.ctx.new_int(fill_one as *const () as usize).into(),
2517+
vm,
2518+
)
2519+
.unwrap();
2520+
scope
2521+
.globals
2522+
.set_item(
2523+
"address_two",
2524+
vm.ctx.new_int(fill_two as *const () as usize).into(),
2525+
vm,
2526+
)
2527+
.unwrap();
2528+
vm.run_string(
2529+
scope.clone(),
2530+
r#"
2531+
import _ctypes
2532+
2533+
class CInt(_ctypes._SimpleCData):
2534+
_type_ = "i"
2535+
2536+
events = []
2537+
fail = False
2538+
2539+
class Out(CInt):
2540+
def __ctypes_from_outparam__(self):
2541+
events.append(("convert", self.value))
2542+
if fail:
2543+
raise ValueError("output conversion failed")
2544+
return ("converted", self.value)
2545+
2546+
class Pointer(_ctypes._Pointer):
2547+
_type_ = Out
2548+
2549+
class Raw(CInt):
2550+
def __del__(self):
2551+
events.append("drop raw result")
2552+
2553+
class One(_ctypes.CFuncPtr):
2554+
_flags_ = 1
2555+
_argtypes_ = (Pointer,)
2556+
_restype_ = Raw
2557+
2558+
class Two(_ctypes.CFuncPtr):
2559+
_flags_ = 1
2560+
_argtypes_ = (Pointer, Pointer)
2561+
_restype_ = Raw
2562+
2563+
out_one = One(address_one)
2564+
inout_one = One(address_one)
2565+
out_two = Two(address_two)
2566+
flags_one = ((2, "first"),)
2567+
flags_inout = ((3, "first"),)
2568+
flags_two = ((2, "first"), (2, "second"))
2569+
"#,
2570+
"<ctypes native output fixture>",
2571+
)
2572+
.unwrap();
2573+
// The ordinary DLL constructor currently discards paramflags.
2574+
// Inject only that metadata, as the COM constructor does, then
2575+
// exercise the real Python -> libffi -> Python call path.
2576+
for (function, flags) in [
2577+
("out_one", "flags_one"),
2578+
("inout_one", "flags_inout"),
2579+
("out_two", "flags_two"),
2580+
] {
2581+
let function = scope
2582+
.globals
2583+
.get_item(function, vm)
2584+
.unwrap()
2585+
.downcast::<PyCFuncPtr>()
2586+
.unwrap();
2587+
*function.paramflags.write() = Some(scope.globals.get_item(flags, vm).unwrap());
2588+
}
2589+
vm.run_string(
2590+
scope,
2591+
r#"
2592+
assert out_one() == ("converted", 11)
2593+
assert events == ["drop raw result", ("convert", 11)]
2594+
events.clear()
2595+
value = Out()
2596+
assert inout_one(value) is value
2597+
assert value.value == 11
2598+
assert events == ["drop raw result"]
2599+
events.clear()
2600+
assert out_two() == (("converted", 11), ("converted", 22))
2601+
assert events == ["drop raw result", ("convert", 11), ("convert", 22)]
2602+
events.clear()
2603+
out_one.errcheck = lambda result, function, args: None
2604+
assert out_one() is None
2605+
assert events == ["drop raw result"]
2606+
events.clear()
2607+
out_one.errcheck = lambda result, function, args: args
2608+
assert out_one() == ("converted", 11)
2609+
assert events == ["drop raw result", ("convert", 11)]
2610+
events.clear()
2611+
fail = True
2612+
try:
2613+
out_one()
2614+
except ValueError as error:
2615+
assert str(error) == "output conversion failed"
2616+
else:
2617+
raise AssertionError("output conversion failure was swallowed")
2618+
assert events == ["drop raw result", ("convert", 11)]
2619+
"#,
2620+
"<ctypes native output assertions>",
2621+
)
2622+
.unwrap();
2623+
});
2624+
}
2625+
24942626
#[test]
24952627
fn unlocked_raw_buffers_are_copied_even_when_readonly() {
24962628
crate::Interpreter::without_stdlib(Default::default()).enter(|vm| {

0 commit comments

Comments
 (0)