Handle UuidToStringA failure in GenerateRuntimeId - #9026
Draft
link04 wants to merge 1 commit into
Draft
Conversation
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Execution-Time Benchmarks Report ⏱️Execution-time results for samples comparing This PR (9026) and master.
|
|||||||||||||||||||||||||||||||||||
| Metric | Master (Mean ± 95% CI) | Current (Mean ± 95% CI) | Change | Status |
|---|---|---|---|---|
| .NET Framework 4.8 - Baseline | ||||
| duration | 190.44 ± (190.41 - 191.25) ms | 211.75 ± (211.51 - 212.43) ms | +11.2% | ❌⬆️ |
| .NET Framework 4.8 - Bailout | ||||
| duration | 194.15 ± (194.07 - 194.52) ms | 217.89 ± (217.34 - 218.42) ms | +12.2% | ❌⬆️ |
| .NET Framework 4.8 - CallTarget+Inlining+NGEN | ||||
| duration | 1168.63 ± (1168.89 - 1174.84) ms | 1293.36 ± (1291.05 - 1298.29) ms | +10.7% | ❌⬆️ |
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
1 participant
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 of changes
GenerateRuntimeIdignored the return value ofUuidToStringAand freed the output pointer unconditionally. Check it, and fall back toGenerateUuidV4()when it fails.Reason for change
UuidToStringAreturnsRPC_S_OUT_OF_MEMORYon failure and leavesStringUuiduntouched.strwas uninitialised, so on that path we handed a garbage stack value toRpcStringFreeA. That reachesRtlFreeHeapwith a pointer that was never a live heap block, and Windows treats it asSTATUS_HEAP_CORRUPTION, which is a non-continuable fast-fail. The process dies immediately, with no chance to handle it.This runs on essentially every instrumented .NET process start on Windows.
RuntimeIdStoreis a by-value member of the native loader'sCorProfiler, so its constructor runs duringCoCreateProfiler. A rare allocation failure inside RPC therefore becomes a hard kill of the customer's process rather than a degraded runtime id.I found this while looking at a crash report with exactly that signature on 3.32.1:
runtimeid_store.cpp:25->RpcStringFree->RtlFreeHeap->RtlpHeapHandleError->RtlReportCriticalFailure. To be clear, I can't prove that report was caused by this rather than by heap corruption elsewhere in the process that this free happened to be the first to detect. The unchecked return value is wrong either way, so this is worth fixing on its own merits.Implementation details
Initialise
str, and return the existingGenerateUuidV4()fallback whenUuidToStringAdoes not returnRPC_S_OK.GenerateUuidV4is already the non-Windows implementation, so there is no new code behind the fallback.Not checking
UuidCreateon purpose.RPC_S_UUID_LOCAL_ONLYstill yields a usable UUID, and that result has no bearing on the free.Test coverage
The existing
runtimeid_storegtests only cover the success path. Covering the failure path needs a seam to inject theUuidToStringAresult, which I left out to keep this small. Happy to add it if reviewers would rather have it.I could not build this locally, so CI is the first compile check.
Other details
Windows only. Linux already went through
GenerateUuidV4().🤖 Generated with Claude Code