Skip to content

Commit fecd913

Browse files
committed
Address review comments
1 parent b819453 commit fecd913

File tree

3 files changed

+24
-6
lines changed

3 files changed

+24
-6
lines changed

onnxruntime/python/onnxruntime_pybind_mlvalue.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,12 @@ void CreateGenericMLValue(const onnxruntime::InputDefList* input_def_list, const
127127
bool accept_only_numpy_array = false, bool use_numpy_data_memory = true,
128128
const MemCpyFunc& mem_cpy_to_device = CpuToCpuMemCpy);
129129

130+
/// @param zero_copy_non_owning When false (default), CPU tensors that do not own
131+
/// their buffer are copied to a new numpy array to prevent dangling pointers
132+
/// (e.g. when a model output aliases a numpy input array). Set to true ONLY
133+
/// when the caller explicitly manages the backing memory lifetime — currently
134+
/// this is limited to OrtValue.numpy(), where the user holds the OrtValue
135+
/// Python object and is responsible for keeping it alive.
130136
pybind11::object GetPyObjFromTensor(const OrtValue& rtensor,
131137
const DataTransferManager* data_transfer_manager = nullptr,
132138
const std::unordered_map<OrtDevice, MemCpyFunc>* mem_cpy_to_host_functions = nullptr,

onnxruntime/python/onnxruntime_pybind_ortvalue.cc

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -419,6 +419,10 @@ void addOrtValueMethods(pybind11::module& m) {
419419
/*zero_copy_non_owning=*/true);
420420
#endif
421421
default:
422+
// OrtValue.numpy() is called by the user who explicitly holds the OrtValue
423+
// Python object, so the backing memory lifetime is managed externally.
424+
// zero_copy_non_owning=true is safe here (and required to preserve the
425+
// zero-copy semantics that OrtValue.numpy() / __array__ rely on).
422426
return GetPyObjFromTensor(*ml_value, nullptr, nullptr,
423427
/*zero_copy_non_owning=*/true);
424428
}

onnxruntime/test/python/onnxruntime_test_python.py

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import copy
66
import ctypes
77
import gc
8+
import importlib.util
89
import os
910
import pathlib
1011
import platform
@@ -1514,29 +1515,32 @@ def test_ort_value_array_protocol(self):
15141515
np.testing.assert_equal(bool_arr, result_bool)
15151516
self.assertEqual(result_bool.dtype, np.bool_)
15161517

1518+
@unittest.skipUnless(
1519+
importlib.util.find_spec("onnx") is not None,
1520+
"onnx package is required to build the test model",
1521+
)
15171522
def test_run_output_does_not_alias_input_passthrough(self):
15181523
"""Test that session.run() returns independent numpy arrays when a model
15191524
input passes through as a model output. Reproduces the dangling-pointer
15201525
corruption described in https://github.com/microsoft/onnxruntime/issues/21922
15211526
"""
15221527
import onnx # noqa: PLC0415
1523-
from onnx import TensorProto, helper # noqa: PLC0415
15241528

15251529
# Build a model where 'input_0' is both a graph input and a graph output,
15261530
# plus a computed output (input_0 + 10).
15271531
inp_shape = [1, 2, 2, 2]
1528-
input_0 = helper.make_tensor_value_info("input_0", TensorProto.FLOAT, inp_shape)
1529-
output_plus10 = helper.make_tensor_value_info("plus_10", TensorProto.FLOAT, inp_shape)
1532+
input_0 = onnx.helper.make_tensor_value_info("input_0", onnx.TensorProto.FLOAT, inp_shape)
1533+
output_plus10 = onnx.helper.make_tensor_value_info("plus_10", onnx.TensorProto.FLOAT, inp_shape)
15301534
ten_const = onnx.numpy_helper.from_array(np.array(10, dtype=np.float32), "ten_const")
1531-
add_node = helper.make_node("Add", ["input_0", "ten_const"], ["plus_10"], name="Add0")
1532-
graph = helper.make_graph(
1535+
add_node = onnx.helper.make_node("Add", ["input_0", "ten_const"], ["plus_10"], name="Add0")
1536+
graph = onnx.helper.make_graph(
15331537
[add_node],
15341538
"PassthroughTest",
15351539
[input_0],
15361540
[output_plus10, input_0],
15371541
initializer=[ten_const],
15381542
)
1539-
model = helper.make_model(graph, opset_imports=[helper.make_opsetid("", 21)])
1543+
model = onnx.helper.make_model(graph, opset_imports=[onnx.helper.make_opsetid("", 21)])
15401544
model = onnx.shape_inference.infer_shapes(model)
15411545

15421546
sess_options = onnxrt.SessionOptions()
@@ -1567,6 +1571,10 @@ def test_run_output_does_not_alias_input_passthrough(self):
15671571

15681572
# The pass-through output must NOT alias the input buffer —
15691573
# it must be an independent copy so it survives across runs.
1574+
self.assertFalse(
1575+
np.shares_memory(outputs[1], input_data),
1576+
f"Run {run_index}: 'input_0' unexpectedly aliases the input buffer",
1577+
)
15701578
all_run_outputs.append(outputs)
15711579

15721580
# After all runs, every saved output must still hold its original value.

0 commit comments

Comments
 (0)