Fix OrtGlobals::Allocator destruction order - #2249
Conversation
OrtGlobals::Allocator declared `allocator_` before `session_`, so destruction ran in the wrong order: `session_` was destroyed first, invalidating the session-scoped allocator per OrtApi::CreateAllocator's documented contract ("the allocator wraps the internal allocator from the OrtSession and becomes invalid when the session does"). ~allocator_ then released an invalid handle, crashing with INVALID_POINTER_READ_AVRF inside the ORT plugin-EP deleter lambda at shutdown.
Reproduced deterministically by the Windows ML EP cert tool against the WebGPU EP under App Verifier (12 models / 53 tasks, 3 LLM AVRF failures). With this one-line swap and no other changes, all 53/53 tasks pass; all 10 app-verifier tasks succeed.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- ort_genai_c.h: expand OgaShutdown doxygen to warn that skipping the explicit shutdown can crash (globals destroyed in undefined static-destruction order), point C++/C# callers at the OgaHandle wrappers, and note that all GenAI handles must be released first. - ort_genai.h: add a docstring on OgaHandle mirroring the OgaShutdown structure, including a note that only one OgaHandle should be live in the process since GenAI's globals are not re-creatable after OgaShutdown(). - models/onnxruntime_api.h: document Allocator::Create's lifetime constraint (becomes invalid when the OrtSession is destroyed). This is the contract whose violation caused the destruction-order bug fixed in the previous commit. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes a shutdown crash caused by incorrect C++ member destruction order in OrtGlobals::Allocator, where an OrtAllocator outlived the OrtSession it depends on (notably crashing with plugin EPs like WebGPU/QNN). It also adds documentation clarifying the required shutdown and allocator/session lifetime contracts across the C, C++, and internal ORT wrapper layers.
Changes:
- Fix
OrtGlobals::Allocatormember declaration order so the session outlives the session-scoped allocator. - Document
OgaShutdown()/OgaHandleexpectations to encourage explicit shutdown and correct handle lifetime usage. - Document
Allocator::Createlifetime constraints in the ORT wrapper API (onnxruntime_api.h).
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/generators.h | Swaps session_/allocator_ declaration order and adds rationale/comment to prevent allocator release after session teardown. |
| src/models/onnxruntime_api.h | Documents that allocators created from a session become invalid when the session is destroyed. |
| src/ort_genai_c.h | Expands OgaShutdown() doxygen: explicit shutdown recommendation + ordering constraints for callers. |
| src/ort_genai.h | Adds OgaHandle RAII documentation warning about shutdown ordering and non-recreatable globals. |
Comments suppressed due to low confidence (1)
src/ort_genai.h:866
OgaHandleis copyable/movable, so accidental pass-by-value or container use can create multiple instances whose destructors each callOgaShutdown(). That contradicts the doc comment (“Only one OgaHandle should be live”) and can trigger double-shutdown paths (e.g.,Generators::Shutdown()/ interface shutdown) in an unsupported order. Make the wrapper non-copyable/non-movable to enforce single ownership semantics.
struct OgaHandle {
OgaHandle() = default;
~OgaHandle() noexcept {
OgaShutdown();
}
|
Technically we don't need the global sessions with plugin EPs as their allocators can come from the environment. However there are a bunch of questions about what we want to allow around registration/unregistration of a plugin EP, and whether it should be possible to call OgaShutdown and re-init later in the same process. e.g. the FL Manager can be re-created, so it needs to be able to re-init if it calls OgaShutdown in the dtor. I looked into this a little last week and had this WIP plan: https://github.com/microsoft/onnxruntime-genai/blob/f00437ac1dc772e3c4628133a5a25839c62bec6a/docs/PluginEPSharedAllocator.md |
## Summary `OrtGlobals::Allocator` in `src/generators.h` declares `allocator_` before `session_`. Because C++ destroys non-static members in reverse declaration order, `session_` is destroyed first, which invalidates the session-scoped `OrtAllocator` per ORT's documented contract on `OrtApi::CreateAllocator`: > The allocator wraps the internal allocator from the OrtSession and becomes invalid when the session does. The subsequent `~allocator_` then calls `OrtApi::ReleaseAllocator` against the now-invalid handle. For plugin EPs (WebGPU, QNN, …), this drops into the ORT plugin-EP deleter lambda whose `[this]` capture is a dangling `PluginExecutionProvider*`, crashing with `INVALID_POINTER_READ_AVRF` at shutdown. **Fix:** one-line declaration-order swap so `session_` is declared first and destroyed last. The fix is in the first commit; the second commit is doc-only. ## Why this surfaces now Three conditions are needed to make the crash deterministic: 1. **A plugin EP** (WebGPU/QNN) — uses the lambda-capture deleter on the ORT side. 2. **Explicit `OgaShutdown()`** before process exit (the recommended pattern from #311). This moves `~OrtGlobals::Allocator` out of `LdrShutdownProcess` noise into a clean point in the process lifetime. 3. **App Verifier page-heap** active. Without it, the dangling read returns stale memory and the process probably exits while silently corrupting heap. ## Validation Reproduced and verified end-to-end with the WebGPU plugin EP under App Verifier: deterministic AVRF crashes at shutdown before the fix → clean shutdown after. GenAI unit tests on this branch (local CPU-only Release build) — shutdown path exercised cleanly: ``` [==========] 74 tests from 8 test suites ran. (4060 ms total) [ PASSED ] 53 tests. [ SKIPPED ] 21 tests Shutting down OnnxRuntime... done ``` The defect exists identically at `v0.12.1` (shipped) and at `main` HEAD. ## Minimal ORT-only reproducer (no GenAI) The dangling-pointer read can be hit using only the ORT C API by deliberately misusing the `CreateAllocator` lifetime contract against any plugin EP: ```cpp RegisterExecutionProviderLibrary(env, "WebGpuExecutionProvider", dll_path); SessionOptionsAppendExecutionProvider_V2(opts, env, &device, 1, nullptr, nullptr, 0); CreateSession(env, model_path, opts, &session); const OrtMemoryInfo* mi = EpDevice_MemoryInfo(device, OrtDeviceMemoryType_DEFAULT); OrtAllocator* leaked = nullptr; CreateAllocator(session, mi, &leaked); ReleaseSession(session); // destroys PluginExecutionProvider ReleaseAllocator(leaked); // AVRF in plugin-EP deleter lambda ``` This is what GenAI does at shutdown, just spread across `~OrtGlobals::Allocator`. ## Changes **Commit 1 — Fix OrtGlobals::Allocator destruction order** - `src/generators.h`: swap declaration order of `allocator_` and `session_`; add a comment explaining why field order matters here, with a permalink to ORT's `OrtApi::CreateAllocator` contract. **Commit 2 — Document OgaShutdown / OgaHandle / Allocator::Create lifetime contracts** (doc-only) - `src/ort_genai_c.h`: expand `OgaShutdown` doxygen to warn that skipping the explicit shutdown can crash (globals destroyed in undefined static-destruction order); direct C++/C# callers to the `OgaHandle` wrappers. Motivated by the rationale given in #311, which introduced `OgaShutdown()`. - `src/ort_genai.h`: add a docstring on `OgaHandle` mirroring the `OgaShutdown` structure, including a `\note` that only one `OgaHandle` should be live in the process since GenAI's globals are not re-creatable after `OgaShutdown()`. - `src/models/onnxruntime_api.h`: document `Allocator::Create`'s lifetime constraint — the Allocator becomes invalid when the OrtSession is destroyed. This is the contract whose violation caused the bug fixed in the first commit. The doc commit is independently revertable if you'd prefer to merge only the fix. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Summary
OrtGlobals::Allocatorinsrc/generators.hdeclaresallocator_beforesession_. Because C++ destroys non-static members in reverse declaration order,session_is destroyed first, which invalidates the session-scopedOrtAllocatorper ORT's documented contract onOrtApi::CreateAllocator:The subsequent
~allocator_then callsOrtApi::ReleaseAllocatoragainst the now-invalid handle. For plugin EPs (WebGPU, QNN, …), this drops into the ORT plugin-EP deleter lambda whose[this]capture is a danglingPluginExecutionProvider*, crashing withINVALID_POINTER_READ_AVRFat shutdown.Fix: one-line declaration-order swap so
session_is declared first and destroyed last. The fix is in the first commit; the second commit is doc-only.Why this surfaces now
Three conditions are needed to make the crash deterministic:
OgaShutdown()before process exit (the recommended pattern from Fix ort global variable destruction order #311). This moves~OrtGlobals::Allocatorout ofLdrShutdownProcessnoise into a clean point in the process lifetime.Validation
Reproduced and verified end-to-end with the WebGPU plugin EP under App Verifier: deterministic AVRF crashes at shutdown before the fix → clean shutdown after.
GenAI unit tests on this branch (local CPU-only Release build) — shutdown path exercised cleanly:
The defect exists identically at
v0.12.1(shipped) and atmainHEAD.Minimal ORT-only reproducer (no GenAI)
The dangling-pointer read can be hit using only the ORT C API by deliberately misusing the
CreateAllocatorlifetime contract against any plugin EP:This is what GenAI does at shutdown, just spread across
~OrtGlobals::Allocator.Changes
Commit 1 — Fix OrtGlobals::Allocator destruction order
src/generators.h: swap declaration order ofallocator_andsession_; add a comment explaining why field order matters here, with a permalink to ORT'sOrtApi::CreateAllocatorcontract.Commit 2 — Document OgaShutdown / OgaHandle / Allocator::Create lifetime contracts (doc-only)
src/ort_genai_c.h: expandOgaShutdowndoxygen to warn that skipping the explicit shutdown can crash (globals destroyed in undefined static-destruction order); direct C++/C# callers to theOgaHandlewrappers. Motivated by the rationale given in Fix ort global variable destruction order #311, which introducedOgaShutdown().src/ort_genai.h: add a docstring onOgaHandlemirroring theOgaShutdownstructure, including a\notethat only oneOgaHandleshould be live in the process since GenAI's globals are not re-creatable afterOgaShutdown().src/models/onnxruntime_api.h: documentAllocator::Create's lifetime constraint — the Allocator becomes invalid when the OrtSession is destroyed. This is the contract whose violation caused the bug fixed in the first commit.The doc commit is independently revertable if you'd prefer to merge only the fix.