Skip to content

Lazily import onnx so a broken onnx does not block 'import monai' (#8455) #8937

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.

Sign up for GitHub

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

Open
Shizoqua wants to merge 1 commit into
base: dev
Choose a base branch
from
+68 −4
Open

Lazily import onnx so a broken onnx does not block 'import monai' (#8455) #8937

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion monai/bundle/scripts.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@
ValidationError, _ = optional_import("jsonschema.exceptions", name="ValidationError")
Checkpoint, has_ignite = optional_import("ignite.handlers", IgniteInfo.OPT_IMPORT_VERSION, min_version, "Checkpoint")
requests, has_requests = optional_import("requests")
onnx, _ = optional_import("onnx")
huggingface_hub, _ = optional_import("huggingface_hub")

logger = get_logger(module_name=__name__)
Expand Down Expand Up @@ -1419,6 +1418,7 @@ def onnx_export(
converter_kwargs_.update({"inputs": inputs_, "use_trace": use_trace_})

def save_onnx(onnx_obj: Any, filename_prefix_or_stream: str, **kwargs: Any) -> None:
onnx, _ = optional_import("onnx")
onnx.save(onnx_obj, filename_prefix_or_stream)

_export(
Expand Down
7 changes: 4 additions & 3 deletions monai/networks/utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,6 @@
from monai.utils.module import look_up_option, optional_import
from monai.utils.type_conversion import convert_to_dst_type, convert_to_tensor

onnx, _ = optional_import("onnx")
onnxreference, _ = optional_import("onnx.reference")
onnxruntime, _ = optional_import("onnxruntime")
polygraphy, polygraphy_imported = optional_import("polygraphy")
torch_tensorrt, _ = optional_import("torch_tensorrt", "1.4.0")

Expand Down Expand Up @@ -708,6 +705,8 @@ def convert_to_onnx(
https://pytorch.org/docs/master/generated/torch.jit.script.html.

"""
onnx, _ = optional_import("onnx")

model.eval()
with torch.no_grad():
torch_versioned_kwargs = {}
Expand Down Expand Up @@ -777,11 +776,13 @@ def convert_to_onnx(
model_input_names = [i.name for i in onnx_model.graph.input]
input_dict = dict(zip(model_input_names, [i.cpu().numpy() for i in inputs]))
if use_ort:
onnxruntime, _ = optional_import("onnxruntime")
ort_sess = onnxruntime.InferenceSession(
onnx_model.SerializeToString(), providers=ort_provider if ort_provider else ["CPUExecutionProvider"]
)
onnx_out = ort_sess.run(None, input_dict)
else:
onnxreference, _ = optional_import("onnx.reference")
sess = onnxreference.ReferenceEvaluator(onnx_model)
onnx_out = sess.run(None, input_dict)
set_determinism(seed=None)
Expand Down
63 changes: 63 additions & 0 deletions tests/networks/test_lazy_onnx_import.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
# Copyright (c) MONAI Consortium
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
# http://www.apache.org/licenses/LICENSE-2.0
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

from __future__ import annotations

import subprocess
import sys
import unittest

# Run in a subprocess with a meta-path finder that records any import *attempt*
# of the optional heavy backends (works whether or not they are installed, and
# does not shadow/replace them). After importing MONAI, fail if any were touched.
_PROBE = """
import sys, importlib.abc
_WATCH = ("onnx", "onnxruntime")
_attempted = set()


class _Recorder(importlib.abc.MetaPathFinder):
def find_spec(self, name, path, target=None):
if name.split(".")[0] in _WATCH:
_attempted.add(name.split(".")[0])
return None # delegate to the real finders


sys.meta_path.insert(0, _Recorder())
{import_stmt}
sys.exit("eagerly imported: " + ", ".join(sorted(_attempted)) if _attempted else 0)
"""


class TestLazyOptionalImports(unittest.TestCase):
"""Regression tests for #8455.

Importing MONAI must not eagerly import onnx / onnxruntime. These were
previously imported at module scope via ``optional_import`` in
``monai/networks/utils.py`` and ``monai/bundle/scripts.py``, so a
broken/hanging onnx (e.g. onnx 1.18 on Windows) would take down
``import monai`` entirely.
"""

def _assert_not_eagerly_imported(self, import_stmt: str) -> None:
code = _PROBE.format(import_stmt=import_stmt)
result = subprocess.run([sys.executable, "-c", code], capture_output=True, text=True)
self.assertEqual(result.returncode, 0, msg=(result.stdout + result.stderr).strip())
Comment on lines +50 to +53

@coderabbitai coderabbitai Bot Jun 20, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
python - <<'PY'
import ast, pathlib
p = pathlib.Path("tests/networks/test_lazy_onnx_import.py")
t = ast.parse(p.read_text())
for n in ast.walk(t):
    if isinstance(n, ast.Call) and isinstance(n.func, ast.Attribute):
        if isinstance(n.func.value, ast.Name) and n.func.value.id == "subprocess" and n.func.attr == "run":
            has_timeout = any(isinstance(k, ast.keyword) and k.arg == "timeout" for k in n.keywords)
            print(f"Line {n.lineno}: timeout_present={has_timeout}")
PY

Repository: Project-MONAI/MONAI

Length of output: 94

🏁 Script executed:

cat -n tests/networks/test_lazy_onnx_import.py

Repository: Project-MONAI/MONAI

Length of output: 2879

🏁 Script executed:

#!/bin/bash
python - <<'PY'
import ast, pathlib
p = pathlib.Path("tests/networks/test_lazy_onnx_import.py")
content = p.read_text()
t = ast.parse(content)

print("=== ALL subprocess.run CALLS ===")
for n in ast.walk(t):
    if isinstance(n, ast.Call) and isinstance(n.func, ast.Attribute):
        if isinstance(n.func.value, ast.Name) and n.func.value.id == "subprocess" and n.func.attr == "run":
            has_timeout = any(isinstance(k, ast.keyword) and k.arg == "timeout" for k in n.keywords)
            print(f"Line {n.lineno}: has_timeout={has_timeout}")

print("\n=== DOCSTRINGS IN METHODS ===")
for n in ast.walk(t):
    if isinstance(n, (ast.FunctionDef, ast.AsyncFunctionDef)):
        has_docstring = (
            n.body and isinstance(n.body[0], ast.Expr) and isinstance(n.body[0].value, ast.Constant)
        )
        print(f"Line {n.lineno}: {n.name} - has_docstring={has_docstring}")
PY

Repository: Project-MONAI/MONAI

Length of output: 361

Add timeout to subprocess probe and docstrings to test methods.

Line 52 subprocess.run lacks a timeout parameter—if import monai hangs again, this test blocks the suite indefinitely. Additionally, the helper method _assert_not_eagerly_imported and test methods at lines 55–58 require docstrings per PEP257 guidelines.

Suggested patch 
 def _assert_not_eagerly_imported(self, import_stmt: str) -> None:
     code = _PROBE.format(import_stmt=import_stmt)
-    result = subprocess.run([sys.executable, "-c", code], capture_output=True, text=True)
+    result = subprocess.run([sys.executable, "-c", code], capture_output=True, text=True, timeout=30)
     self.assertEqual(result.returncode, 0, msg=(result.stdout + result.stderr).strip())
🧰 Tools
🪛 ast-grep (0.43.0)

[error] 51-51: Command coming from incoming request
Context: subprocess.run([sys.executable, "-c", code], capture_output=True, text=True)
Note: [CWE-20].

(subprocess-from-request)

🪛 Ruff (0.15.17)

[error] 52-52: subprocess call: check for execution of untrusted input

(S603)

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/networks/test_lazy_onnx_import.py` around lines 50 - 53, Add a timeout
parameter to the subprocess.run call in the _assert_not_eagerly_imported method
to prevent the test suite from hanging indefinitely if the import operation
stalls. Additionally, add PEP257-compliant docstrings to the
_assert_not_eagerly_imported helper method and to all test methods in the test
class (around lines 55-58) that currently lack them, ensuring each docstring
briefly describes what the method does.


def test_import_monai_does_not_import_onnx(self):
self._assert_not_eagerly_imported("import monai")

def test_import_monai_bundle_does_not_import_onnx(self):
self._assert_not_eagerly_imported("import monai.bundle")


if __name__ == "__main__":
unittest.main()
Loading