Refactor process.cpp to eliminate vector dependency and shutdown race conditions #123074
+56
−45
Conversation
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
Co-authored-by: jkotas <[email protected]>
Co-authored-by: jkotas <[email protected]>
Co-authored-by: jkotas <[email protected]>
…additions Co-authored-by: jkotas <[email protected]>
jkotas
reviewed
Jan 11, 2026
Copilot
AI
changed the title
[WIP] Refactor process.cpp to remove vector dependency
Refactor process.cpp to eliminate vector dependency and shutdown race conditions
Jan 11, 2026
Changed runtime bounds checks to _ASSERTE calls since MAX_ARGV_ENTRIES (32) is large enough that these conditions should never be true in practice. This matches the pattern used in NativeAOT's PalCreateDump.cpp. Co-authored-by: jkotas <[email protected]>
jkotas
reviewed
Jan 12, 2026
Moved the `argv[argc] = nullptr;` assignment outside the if/else block to eliminate duplication, as suggested by @jkotas. Co-authored-by: jkotas <[email protected]>
jkotas
approved these changes
Jan 14, 2026
Contributor
|
Tagging subscribers to this area: @steveisok, @dotnet/dotnet-diag |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors the crash dump generation code in process.cpp to eliminate the dependency on std::vector and prevent race conditions during process shutdown. The vector's destructor can cause issues when invoked during shutdown, potentially leading to crashes or hangs.
Changes:
- Replaced
std::vector<const char*> g_argvCreateDumpwith fixed-size arrayconst char* g_argvCreateDump[MAX_ARGV_ENTRIES](32 entries) - Updated all related functions to use array indexing instead of vector operations
- Removed
#include <vector>dependency - Added compile-time assertions for bounds checking following the NativeAOT pattern
steveisok
approved these changes
Jan 14, 2026
…NativeAOT with assert pattern - Added bounds check `&& argc < MAX_ARGV_ENTRIES` before processing siginfo in process.cpp - Changed NativeAOT PalCreateDump.cpp to use assert instead of runtime check, consistent with the pattern established in earlier review feedback Co-authored-by: jkotas <[email protected]>
jkotas
reviewed
Jan 14, 2026
Co-authored-by: Copilot <[email protected]>
jkotas
reviewed
Jan 14, 2026
jkotas
reviewed
Jan 14, 2026
jkotas
reviewed
Jan 14, 2026
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.
Fixes potential crashes or hangs during process shutdown due to race conditions in vector destructor execution.
Noticed while reviewing #122989
Description
Replaced
std::vector<const char*> g_argvCreateDumpstatic variable with a fixed-size arrayconst char* g_argvCreateDump[MAX_ARGV_ENTRIES](32 entries) in both CoreCLR and NativeAOT implementations. The vector's destructor can cause race conditions during process shutdown, potentially leading to crashes or hangs.Changes:
const char* g_argvCreateDump[MAX_ARGV_ENTRIES]insrc/coreclr/pal/src/thread/process.cppPROCBuildCreateDumpCommandLineto use array indexing instead ofpush_back()PROCCreateCrashDumpIfEnabledto copy array elements manually and added bounds check for siginfo processingPAL_GenerateCoreDumpto use local fixed-size array#include <vector>dependency from process.cppsrc/coreclr/nativeaot/Runtime/unix/PalCreateDump.cppto use assert instead of runtime check for consistency_ASSERTEfor bounds checking instead of runtime checks, matching the established patternCustomer Impact
Prevents potential crashes or hangs during process shutdown due to race conditions in vector destructor execution, particularly in crash dump generation scenarios.
Regression
No. This addresses a pre-existing race condition.
Testing
CoreCLR and NativeAOT components build successfully with no errors or warnings. Asserts verify that array bounds are respected during development and testing.
Risk
Low. Changes are isolated to crash dump generation code path. Pattern is consistent between CoreCLR and NativeAOT implementations. Asserts will catch any programming errors during development while not adding runtime overhead in release builds.
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.