Fix plugin EP allocator deleter lifetime - #29663
Closed
yen-shi wants to merge 1 commit into
Closed
Conversation
Capture the OrtEpFactory pointer directly in plugin allocator release callbacks instead of capturing transient owner objects. This keeps allocator destruction safe when shared or per-session allocator wrappers outlive the object that created the callback.
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
tianleiwu
approved these changes
Jul 11, 2026
tianleiwu
left a comment
Contributor
There was a problem hiding this comment.
Verdict: Approve
The fix is correct and minimal. Both allocator deleters now capture a stable OrtEpFactory* by value instead of routing through a transient owner (&ep_device in environment.cc, this in the plugin provider), which is exactly the right way to decouple allocator teardown from those objects' lifetimes.
Why this is sound:
- In the plugin provider,
ep_factory_is anOrtEpFactory&reference member, so&ep_factory_points at the externally-owned factory object, not into the provider. Capturing that address by value genuinely survives provider destruction. - The remaining lifetime dependency (factory must outlive the allocator) is already guaranteed for the shared-allocator path:
~Environment()clearsshared_allocators_/shared_ort_allocators_(running the deleters) before tearing downep_libraries_, which own the factories. - Both call sites are fixed identically, so the two allocator-creation paths stay consistent.
Only minor (non-blocking) suggestions inline.
Contributor
|
@yen-shi, could you merge latest main branch to pass CI? |
This was referenced Jul 17, 2026
tianleiwu
added a commit
that referenced
this pull request
Jul 18, 2026
This cherry-picks the following commits for the release: | Commit ID | PR Number | Commit Title | |-----------|-----------|-------------| | dd32f35 | #29590 | Fix libcudart.so.13 hard dependency in pybind module breaking import on CPU-only Linux | | cc44a4d | #29706 | [CUDA] Fix XQA GroupQueryAttention cudaErrorInvalidValue on Blackwell (sm_120) | | 23a7e9d | #29705 | [CUDA] Do not link nvrtc | | ee93f83 | #29711 | [CUDA] Update cuda arch list for packages of cuda 12.8 | | fea45a3 | #29620 | [CUDA] Add cuDNN-free ArgMax/ArgMin/ReduceSum and fix LogSoftmax on plugin EP | | f05b218 | #29624 | Enable Spectre-mitigated MSVC libs for BinSkim builds | | 1c89b86 | #29687 | [BUILD] CUDA_QUANT_PREPROCESS off by default and Adjust CI | | 41bd391 | #29658 | [CUDA] Fix null allocator passed to plugin EP kernel PrePack | | 405fbea | #28896 | Add Windows ARM64 CUDA plugin package and align CUDA metadata/artifact naming | | 308f24c | #29622 | Enable fpA_intB GEMM in CUDA builds and add configurable options | | 16ebc1d | #29731 | [Build] Use GPU pool to unblock CI temporarily | |5911a3a263| #29748 | Add OrtErrorCode::ORT_DEVICE_RESET | |6217f73ec5 | #29663 | Fix plugin EP allocator deleter lifetime | --------- Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: GitHub Copilot <copilot@example.com> Co-authored-by: Edward Chen <18449977+edgchen1@users.noreply.github.com> Co-authored-by: Yen-Shi Wang <yenshiw@nvidia.com>
tianleiwu
added a commit
that referenced
this pull request
Jul 21, 2026
Cherrypick #29663 so that it can pass CI for 1.28 release. ----- ## Summary This PR fixes an AppVerifier failure seen during ONNX Runtime / onnxruntime_genai shutdown with the plugin EP path: ```text INVALID_POINTER_READ_AVRF_c0000005_onnxruntime.dll!OrtGetApiBase Access violation - code c0000005 ``` The bug is in the allocator deleter lifetime. Plugin EP allocators are released later by an `OrtAllocatorUniquePtr` custom deleter, but the deleter was reaching `OrtEpFactory::ReleaseAllocator` through transient owner captures: - `Environment::CreateSharedAllocatorImpl` captured `ep_device` by reference. - `PluginExecutionProvider::CreatePreferredAllocators` captured `this`. The fix captures the required `OrtEpFactory*` directly in both deleters and calls: ```cpp ep_factory->ReleaseAllocator(ep_factory, allocator); ``` This keeps allocator destruction independent from the stack reference / provider object used when the deleter was created, while preserving the same allocator ownership and release API. ## Changed files - `onnxruntime/core/session/environment.cc` - `onnxruntime/core/session/plugin_ep/ep_plugin_provider_interfaces.cc` ## Verification Built ORT and the TRT-RTX EP repro stack against TRT-RTX 1.6.1.114, then ran the DeepSeek WinML AppVerifier repro. - Fixed branch: `"appVerifierEnabled": true`, `"appVerifierFailed": false` - Rebuilt `origin/main` negative control: `"appVerifierEnabled": true`, `"appVerifierFailed": true` This confirms the invalid pointer read still reproduces on `origin/main` and is removed by the allocator deleter lifetime change. Co-authored-by: Yen-Shi Wang <yenshiw@nvidia.com>
Contributor
|
This commit is cherry-picked in #29770 |
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.
Fix plugin EP allocator deleter lifetime
Summary
This PR fixes an AppVerifier failure seen during ONNX Runtime / onnxruntime_genai shutdown with the plugin EP path:
The bug is in the allocator deleter lifetime. Plugin EP allocators are released later by an
OrtAllocatorUniquePtrcustom deleter, but the deleter was reachingOrtEpFactory::ReleaseAllocatorthrough transient owner captures:Environment::CreateSharedAllocatorImplcapturedep_deviceby reference.PluginExecutionProvider::CreatePreferredAllocatorscapturedthis.The fix captures the required
OrtEpFactory*directly in both deleters and calls:ep_factory->ReleaseAllocator(ep_factory, allocator);This keeps allocator destruction independent from the stack reference / provider object used when the deleter was created, while preserving the same allocator ownership and release API.
Changed files
onnxruntime/core/session/environment.cconnxruntime/core/session/plugin_ep/ep_plugin_provider_interfaces.ccVerification
Built ORT and the TRT-RTX EP repro stack against TRT-RTX 1.6.1.114, then ran the DeepSeek WinML AppVerifier repro.
"appVerifierEnabled": true,"appVerifierFailed": falseorigin/mainnegative control:"appVerifierEnabled": true,"appVerifierFailed": trueThis confirms the invalid pointer read still reproduces on
origin/mainand is removed by the allocator deleter lifetime change.