Repository navigation
Fix optional OpenVINO provider probe without exceptions - #33112
Merged
Mustapha Jaber (mustjab) merged 5 commits intoOct 7, 2026
Merged
Mustapha Jaber (mustjab) merged 5 commits into
Mustapha Jaber (mustjab) merged 5 commits into
Conversation
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The reload test assumes dlclose physically unloads the fixture, causing false failures on loaders that retain it.
Review effort: Balanced
Findings: 1
Open (1)
What changed in this PR
Makes the optional OpenVINO provider probe recover gracefully in exception-disabled builds.
Changes:
- Checks provider load status before calling the aborting getter.
- Adds normal and no-exceptions regression test targets.
- Adds synthetic provider-library fixtures for failure and reload scenarios.
| File | Description |
|---|---|
provider_bridge_ort.cc |
Handles OpenVINO load failures explicitly. |
provider_bridge_ort.h |
Exposes the probe to tests. |
optional_provider_probe_test.cc |
Tests failure, retry, initialization, and reload. |
optional_provider_probe_test_library.cc |
Implements provider test fixtures. |
onnxruntime_unittests.cmake |
Configures fixtures and isolated test targets. |
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Mustapha Jaber (mustjab)
marked this pull request as ready for review
October 5, 2026 21:50
Mustapha Jaber (mustjab)
requested a review
from Sushanth Rajasankar (sushraja-msft)
October 5, 2026 21:50
Mustapha Jaber (mustjab)
enabled auto-merge (squash)
October 5, 2026 21:55
Contributor
Author
|
Fixed the two code/test CI blockers in ae670a2:
Local Windows Release validation: MSVC LTO build and all eight cases passed; all eight also passed after copying both executable directories to a fresh location. Scoped lint, whitespace and editor checks passed. The changes were mirrored into Edge's development checkout before publication; its minimal no-exceptions DLL build and real-DLL native-path smoke checks passed using verified temporary ONNX corrections, which were restored and hash-checked afterward. This is development validation, not pristine published-package validation. The push starts new-head CI. Existing unrelated macOS runner and iOS dependency-download failures were not patched around. Linux/macOS runtime tests were not run locally. Implemented and validated with GitHub Copilot assistance; Mustapha Jaber (@mustjab) owns the change and review follow-through. |
Sushanth Rajasankar (sushraja-msft)
previously approved these changes
Oct 7, 2026
Mustapha Jaber (mustjab)
force-pushed
the
fix/openvino-optional-probe-status
branch
from
October 7, 2026 07:03
ae670a2 to
02ca146
Compare
Sushanth Rajasankar (sushraja-msft)
approved these changes
Oct 7, 2026
eserscor
pushed a commit
that referenced
this pull request
Oct 7, 2026
## Summary `TryGetProviderInfo_OpenVINO()` is an optional probe that should log and return `nullptr` when its provider library is unavailable. It currently calls `ProviderLibrary::Get()`, which uses `ORT_THROW_IF_ERROR(Load())`. With `ORT_NO_EXCEPTIONS`, that macro aborts instead of reaching the catch-and-return path. Check the library's load status before calling the reference getter. Preserve runtime-error telemetry and the detailed error log, and return `nullptr` on a failed status. Required getters and provider initialization semantics remain unchanged. Add isolated native tests for repeated missing shared/provider libraries, missing `GetProvider` export cleanup and replacement, and successful initialization/reload. The fixtures use the real `Provider` interface and require no OpenVINO installation. The no-exceptions target recompiles the actual bridge and error-message helper with compiler exceptions disabled, including ONNX's no-exceptions definition. A compile-time check rejects accidentally enabled exception unwinding. ## Validation - Windows Release MSVC build and all eight regression cases passed across normal and compiler-no-exceptions targets. - Edge clang-cl C++20 syntax check of the actual bridge passed with `-fno-exceptions`, `ORT_NO_EXCEPTIONS`, and `ONNX_NO_EXCEPTIONS`. - Scoped lint, formatting, and whitespace checks passed. The isolated no-exceptions target links the normal ORT test libraries; this is not a globally no-exceptions build of every dependency. Linux, macOS, and AIX runtime tests have not been run locally.
Mustapha Jaber (mustjab)
added a commit
that referenced
this pull request
Oct 14, 2026
## Description Fixes #33325. `ProviderInfo_OpenVINO` is a global interface declared by the OpenVINO provider factory header. #33112 added a forward declaration inside `namespace onnxruntime`, creating a different type. That conflicts with the existing Python binding declaration of `TryGetProviderInfo_OpenVINO` and causes MSVC C2556 when building with OpenVINO and Python enabled. Move the interface forward declaration to global scope while retaining the function in `namespace onnxruntime`. Make the existing optional-provider probe test use an explicitly global interface pointer so a namespace mismatch fails compilation. ## Validation - Windows Visual Studio 18 2026: built the optional-provider probe targets in normal and no-exceptions modes. - Both corresponding CTest suites passed. - clang-cl declaration probes passed in both provider-bridge/Python-binding include orders. - Scoped lintrunner and `git diff --check` passed. The full OpenVINO Python wheel build was not run; no OpenVINO SDK was installed for this validation.
adrastogi
added a commit
that referenced
this pull request
Oct 27, 2026
### Description <!-- Describe your changes. --> This cherry-picks the following commits for the release: * #32610 * #32633 * #32641 * #32611 * #32609 * #32607 * #32679 * #32681 * #32666 CI fixes: * #32559 (`8ec7981a7439546a54196b9cbc777ae5de708b6b`): guard FP8 XQA test cases by GPU compute capability. The Windows CUDA runner is an SM86 A10, while FP8 XQA requires SM89 or SM90+. INT4/INT8 XQA coverage remains enabled on supported SM80+ devices. This fixes the two FP8 dispatch assertions without changing product behavior. * Raise the Android minimal baseline binary-size threshold by 1 KiB, from 1,589,248 to 1,590,272 bytes (release branch only). CI reports a section total of 1,590,022 bytes, exceeding the previous threshold by 774 bytes. The adjusted threshold leaves 250 bytes of headroom. * #32676, then #33489: rename the old `A10-Test` image and remove the unnecessary image override, in upstream order. * #33492: backport the scale-set runner migration across the release branch's 41 affected workflows. The Linux CUDA, CUDA Plugin, and TensorRT test jobs now use `ort-github-linux-6-a10-scus-ss`. Their previous retries were cancelled by `1es-resource-management[bot]` before any steps executed. The first ten cherry-picks, plus #32676 and #33489, applied cleanly from the original mainline commits, with source commit provenance preserved. #32607 includes the full mainline fix and regression tests rather than the reduced 1.28.3 backport. #33492 required conflict resolution because the release branch has an older workflow layout: * Preserve the existing Linux CI and Linux WebGPU job sets; do not introduce main-only no-exceptions or static-plugin jobs. * Leave `windows_arm64_python.yml` absent. It was introduced after 1.30 branched by #32791; its runner migration does not require adding that workflow to this release. * Preserve Windows WebGPU's combined build/test jobs and their check names, keeping them on the new GPU scale set rather than importing main's split build/test topology. The build-only minimal Edge job uses the new CPU scale set. ### Motivation and Context <!-- - Why is this change required? What problem does it solve? - If it fixes an open issue, please link to the issue here. --> Bring the product fixes from #32806 into ORT 1.30.1, address the Windows CUDA test and Android size-gate failures, and migrate CI runner selection to unblock the cancelled GPU jobs. The merged EP API version correction from #33525 was brought in by merging the updated `rel-1.30.1` base. The `uuid-dev` CI fix in #32491 was already present in the 1.30 base and is not duplicated. The OpenVINO changes in #33112 and #33327 are intentionally excluded: the namespace regression fixed by #33327 was introduced by #33112, and neither change is present in the 1.30 base. Validation: * All nine original product cherry-pick patch IDs were verified against their mainline commits; the additional #32559 patch ID also matches upstream. * The upstream `test_fp8_xqa_cuda_capability` regression passed in an isolated standard-library harness using mocked SM86, SM89, and SM90 capabilities. Additional no-CUDA and external-runner cases passed. * Python syntax and Android threshold consistency/arithmetic checks passed. * `git diff --check 3b0f46f HEAD` passed. * All 41 migrated workflow YAML files parse. Structural comparisons against the pre-migration release workflows confirm that only runner labels, caller pool names, and the reusable pool-input description changed; job sets, steps, triggers, permissions, matrices, and build flags are preserved. * All 13 callers of the reusable Linux build workflow use scale-set pool names; all three previously cancelled GPU test jobs use the new A10 scale set. * The Android release-specific size threshold remains 1,590,272 bytes. * The full helper pytest invocation was not runnable because pytest is not installed in the local virtual environment. No local GPU execution or Android rebuild was performed; CI will validate the integrated changes. --------- Co-authored-by: Wei Wang <wei4.wang@intel.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: shiyi <shiyi.zou@intel.com> Co-authored-by: Bin Miao <bin.miao@intel.com> Co-authored-by: Bryan B <bryan.bernhart@intel.com> Co-authored-by: Xiaofei Han <xiaofeihan@microsoft.com> Co-authored-by: Aditya Rastogi <adityar@ntdev.microsoft.com> Co-authored-by: Tianlei Wu <tlwu@microsoft.com> Co-authored-by: eserscor <erscor@microsoft.com> Copilot-Session: 3eb3fc9b-6147-4706-b7ad-47bb3476377b
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
TryGetProviderInfo_OpenVINO()is an optional probe that should log and returnnullptrwhen its provider library is unavailable. It currently callsProviderLibrary::Get(), which usesORT_THROW_IF_ERROR(Load()). WithORT_NO_EXCEPTIONS, that macro aborts instead of reaching the catch-and-return path.Check the library's load status before calling the reference getter. Preserve runtime-error telemetry and the detailed error log, and return
nullptron a failed status. Required getters and provider initialization semantics remain unchanged.Add isolated native tests for repeated missing shared/provider libraries, missing
GetProviderexport cleanup and replacement, and successful initialization/reload. The fixtures use the realProviderinterface and require no OpenVINO installation. The no-exceptions target recompiles the actual bridge and error-message helper with compiler exceptions disabled, including ONNX's no-exceptions definition. A compile-time check rejects accidentally enabled exception unwinding.Validation
-fno-exceptions,ORT_NO_EXCEPTIONS, andONNX_NO_EXCEPTIONS.The isolated no-exceptions target links the normal ORT test libraries; this is not a globally no-exceptions build of every dependency. Linux, macOS, and AIX runtime tests have not been run locally.