Skip to content

Build model_package without exceptions - #32595

Closed
Ryan VanderMeulen (rvandermeulen) wants to merge 1 commit into
microsoft:mainfrom
rvandermeulen:model-package-no-exceptions
Closed

Build model_package without exceptions#32595
Ryan VanderMeulen (rvandermeulen) wants to merge 1 commit into
microsoft:mainfrom
rvandermeulen:model-package-no-exceptions

Conversation

@rvandermeulen

@rvandermeulen Ryan VanderMeulen (rvandermeulen) commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

Description

Add a ParseJsonNoThrow helper next to the ordered_json alias in model_package_impl.h and route the three JSON parse sites through it. With exceptions enabled it is the existing try/catch and keeps the parser's message in the returned status, so nothing changes for regular builds. With exceptions disabled it uses nlohmann's non-throwing parse overload, which reports no detail but returns instead of aborting. The schema_version digits parse moves from std::stoll to std::from_chars, which needs no exception handling in either mode.

Verified by building the standalone library with MODEL_PACKAGE_BUILD_TESTS=ON both normally and with -fno-exceptions; all four test binaries pass in both configurations.

Motivation and Context

model_package deliberately has no dependency on ONNX Runtime, so it cannot use ORT_TRY/ORT_CATCH, and its raw try/catch blocks break an onnxruntime_DISABLE_EXCEPTIONS build since the library is linked into onnxruntime_session. Under -fno-exceptions nlohmann's throwing parse also compiles to std::abort() on malformed input, so the non-throwing overload is the only way to report a bad manifest as a status in that configuration. Firefox builds ONNX Runtime without exceptions and currently carries this as a local patch.

Copilot AI balanced review requested due to automatic review settings September 14, 2026 19:22
@azure-pipelines

azure-pipelines Bot commented Sep 14, 2026

Copy link
Copy Markdown
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

Merged
Copilot AI reviewed Sep 14, 2026
View reviewed changes

Copilot AI left a comment

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.

🟡 Changes recommended

The helper can still abort when nlohmann exceptions are disabled independently of compiler exceptions.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Adds exception-free JSON parsing support to the standalone model package library.

Changes:

  • Adds a shared non-throwing JSON parser helper.
  • Routes manifest, executor-info, and authoring parsing through it.
  • Replaces std::stoll with std::from_chars.
File summaries
File Description
model_package/src/model_package_impl.h Adds JSON parsing helper.
model_package/src/manifest_parser.cc Uses helper and exception-free version parsing.
model_package/src/authoring.cc Uses helper for authoring JSON inputs.
Review details
  • Files reviewed: 3/3 changed files
  • Comments generated: 1
  • Review effort level: Balanced

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread model_package/src/model_package_impl.h Resolved
Copilot AI reviewed Sep 14, 2026
View reviewed changes

Copilot AI left a comment

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.

🟡 Changes recommended

Some nlohmann parser exceptions can still escape the new helper.

Get a fresh assessment by requesting another Copilot review.

Review details
  • Files reviewed: 3/3 changed files
  • Comments generated: 1
  • Review effort level: Balanced

Comment thread model_package/src/model_package_impl.h Resolved
Copilot AI reviewed Sep 14, 2026
View reviewed changes

Copilot AI left a comment

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.

🟢 Approval recommended

The focused changes preserve existing behavior while correctly supporting exception-disabled builds.

Review details
  • Files reviewed: 3/3 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

Merged
@rvandermeulen
Build model_package without exceptions
16be2d6 
model_package deliberately has no dependency on ONNX Runtime, so it cannot use ORT_TRY/ORT_CATCH, and its four raw try/catch blocks break an onnxruntime build with exceptions disabled (onnxruntime_DISABLE_EXCEPTIONS) since the library is linked into onnxruntime_session.

Add a ParseJsonNoThrow helper next to the ordered_json alias: with exceptions enabled it keeps the current behaviour and the parser's message; without them it uses nlohmann's non-throwing parse overload, which would otherwise never be reached because nlohmann's throwing parse aborts the process under -fno-exceptions. The schema_version digits parse moves from std::stoll to std::from_chars, which needs no exception handling in either mode.

Verified by building the standalone library with MODEL_PACKAGE_BUILD_TESTS=ON both normally and with -fno-exceptions; all four test binaries pass in both configurations.
@rvandermeulen

Ryan VanderMeulen (rvandermeulen) commented Sep 25, 2026

Copy link
Copy Markdown
Contributor Author

Edward Chen (@edgchen1) This is one of the related PRs from #32597, if you have a chance to take a look.

Edward Chen (edgchen1) pushed a commit that referenced this pull request Sep 28, 2026
@rvandermeulen
Use ORT_TRY/ORT_CATCH for the remaining raw try/catch in the core non…
20ba75f 
…-minimal build (#32597)

### Description
Convert the raw `try`/`catch`/`throw` statements that a CPU-only,
non-minimal build still compiles to the `ORT_TRY` / `ORT_CATCH` /
`ORT_HANDLE_EXCEPTION` / `ORT_RETHROW` / `ORT_THROW` macros the rest of
the code base uses: `graph_partitioner`, `attention_fusion`,
`constant_folding`, `stft_decomposition`, `model_package_context`,
`ep_library_plugin`, `provider_bridge_ort`, and the `ORT_ENABLE_STREAM`
config reader/writer in `allocation_planner` (whose writer already used
`ORT_CATCH` but read the exception outside `ORT_HANDLE_EXCEPTION`).

`provider_bridge_ort.cc`'s 18 function-try-blocks have no macro
equivalent, so each becomes an `ORT_TRY` over the body followed by a
file-local `ORT_CATCH_LOG_RETURN_NULLPTR` that logs and returns
`nullptr`, the tail they all shared. `ProviderLibrary::Load`/`Get` keep
their unload-and-rethrow via `ORT_RETHROW`. The `allocation_planner`
handler that used `EXIT_ON_ERR` is expanded inline so its `return` stays
in the function rather than inside the handler lambda.

With exceptions enabled this is a spelling change with identical control
flow. With `onnxruntime_DISABLE_EXCEPTIONS` the result compiles and
links on Linux x64/aarch64, Windows x64/x86/arm64, macOS x64/arm64 and
Android arm/arm64/x64 (Firefox's CI, with #32591, #32592, #32593 and
#32595 applied alongside).

### Motivation and Context
Firefox builds ONNX Runtime with exceptions disabled and carries these
conversions as local patches, together with a hack that disables
`ORT_ENABLE_STREAM` only because of the `allocation_planner` sites.
Together with the other open PRs this leaves the
`onnxruntime_DISABLE_EXCEPTIONS` + `onnxruntime_MINIMAL_BUILD` coupling
as the only remaining obstacle to a non-minimal no-exceptions build.
Edward Chen (edgchen1) pushed a commit that referenced this pull request Sep 28, 2026
@rvandermeulen
Fix the nlohmann_json tarball hash in the standalone model_package bu…
84cf511 
…ild (#32596)

### Description
Correct the `URL_HASH` in `model_package/CMakeLists.txt` to the SHA256
of the published nlohmann json v3.11.3 `json.tar.xz`. The recorded value
matches only in its first 26 hex digits.

### Motivation and Context
A standalone configure of `model_package` without `NLOHMANN_JSON_DIR`
and without a system `nlohmann_json` takes the `FetchContent` fallback
and fails at download verification with the current hash. Found while
building the library standalone to test #32595.
@rvandermeulen

Ryan VanderMeulen (rvandermeulen) commented Oct 5, 2026

Copy link
Copy Markdown
Contributor Author

Superseded by #32883, which landed the same change. Closing.

@rvandermeulen
Ryan VanderMeulen (rvandermeulen) deleted the model-package-no-exceptions branch October 5, 2026 23:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments

Assignees

@edgchen1 Edward Chen (edgchen1)

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@rvandermeulen @edgchen1