Fix ort global variable destruction order - #311
Merged
Conversation
Baiju Meswani (baijumeswani)
approved these changes
Apr 24, 2024
Baiju Meswani (baijumeswani)
pushed a commit
that referenced
this pull request
May 1, 2024
kunal-vaishnavi
pushed a commit
that referenced
this pull request
Jun 30, 2026
## 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>
Tianlei Wu (tianleiwu)
pushed a commit
that referenced
this pull request
Jul 11, 2026
## 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>
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
2 participants
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.
We are seeing crashes on python exit due to the undefined ordering. This should ensure the allocators are destroyed before the env.
Added a new OgaShutdown() method to the C API that will cleanly exit. If this method isn't called, it's possible to crash during process shutdown as global variables are destroyed in an undefined order (like with cuda on Linux)