Repository navigation
Conversation
`*call.items().begin()` dereferences a temporary iteration_proxy_value, and the structured binding's `name` calls key() on it after the full expression ends. ASan reports stack-use-after-scope in ToolCallStreamAccumulatorTest.RepairedCustomShapesPreserveDuplicateInputSource, which aborts the sanitizer unit run. Read the key and value through an lvalue object iterator, whose references point into the JSON object. Files changed: - sdk_v2/cpp/src/inferencing/generative/toolcalling/tool_call_utils.cc Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 458edacb-429b-4169-99fb-c2241f5113d4
The OgaCreateStringArrayFromStrings and OrtxTensorResultGetAt leaks were fixed by microsoft/onnxruntime-genai#2153 and #2168, and the SDK now consumes GenAI 0.19.0. The full sanitizer suite, including the real-model vision integration tests that drive those image-processor paths, reports no leaks without them. Keeping stale suppressions would hide a regression. Generators::State::Run stays suppressed: its leak still reproduces with GenAI 0.19.0 in StreamingAudioFixture despite #2169, so the comment now records that. Files changed: - sdk_v2/cpp/test/asan/lsan.supp Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 458edacb-429b-4169-99fb-c2241f5113d4
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
No active review issues are recorded for the tool-call lifetime fix and leak-suppression updates.
No open issues 🎉
What changed in this PR
Fixes a lifetime bug in C++ tool-call parsing and updates leak suppressions to reflect GenAI 0.19.0 behavior.
Changes:
- Replaces temporary-backed structured bindings with a JSON iterator to prevent stack-use-after-scope.
- Removes two stale leak suppressions while retaining and documenting the still-reproducing
Generators::State::Runleak.
| File | Description |
|---|---|
sdk_v2/cpp/test/asan/lsan.supp |
Removes resolved leak suppressions and clarifies why the remaining GenAI suppression is needed. |
sdk_v2/cpp/src/inferencing/generative/toolcalling/tool_call_utils.cc |
Uses iterator key/value access to safely parse singleton tool calls. |
Review effort: Balanced
This branch was successfully deployed
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
OgaCreateStringArrayFromStringsandOrtxTensorResultGetAtLSan suppressions. They were fixed by Fix memory leaks onnxruntime-genai#2153 and Cleanup TensorObject created by OrtxTensorResultGetAt onnxruntime-genai#2168, and the SDK now consumes GenAI 0.19.0.Generators::State::Run. Fix nemotron leaks onnxruntime-genai#2169 was merged, but the leak (~2–4 MB per process) still reproduces with GenAI 0.19.0 inStreamingAudioFixture/Nemotron audio tests. The comment now says so.tool_call_utils.cc,const auto& [name, arguments] = *call.items().begin();binds to a temporaryiteration_proxy_value, andname(viakey()) is read after it is destroyed. This abortedfoundry_local_testspartway through under ASan.Validation
Full
scripts/run_sanitizer_tests.py --skip-build --config Debugon Linux (WSL), ASan + UBSan + LSan, ORT 1.31.0 / GenAI 0.19.0, with real models (FOUNDRY_TEST_DATA_DIR):foundry_local_tests: 2777/2785 passed.sdk_integration_tests: 164/164 passed. This includesVisionFixtureandResponsesVisionIntegrationTest(image-processor paths) andStreamingAudioFixture.State::Runtoo produced 2 leak reports rooted inGenerators::State::Run, so it stays.Pre-existing issues on
main, unrelated to this change:ApiBackendTest.ModelHandleDispatchesToHostedAdapterWithoutNativeModelfails in Debug becauseFL_THROWappends a stack trace, soends_with("host connection was lost")is false.IpcChannelTest.DescriptorExhaustionRecoversAndRemainsInterruptibleByClosegets one UBSan "invalid vptr" report in its death-test child. The test exhaustsRLIMIT_NOFILE, and UBSan's vptr check uses a pipe to probe memory accessibility, so this looks like a sanitizer false positive under fd exhaustion.