Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Use per-command option instances in AspNetCore.CommandLine - #9893

#9893
Merged
glen-84 merged 1 commit into
mainChilliCream/graphql-platform:mainfrom
gai/per-instance-command-line-optionsChilliCream/graphql-platform:gai/per-instance-command-line-optionsCopy head branch name to clipboard
Jun 11, 2026
Merged

Use per-command option instances in AspNetCore.CommandLine#9893
glen-84 merged 1 commit into
mainChilliCream/graphql-platform:mainfrom
gai/per-instance-command-line-optionsChilliCream/graphql-platform:gai/per-instance-command-line-optionsCopy head branch name to clipboard

Conversation

@glen-84

@glen-84 glen-84 commented Jun 11, 2026

Copy link
Copy Markdown
Member

Summary

System.CommandLine symbols (Command, Option, Argument) are intentionally not thread-safe per the maintainers (dotnet/command-line-api#1258). The Opt<TOption> static singleton helper in AspNetCore.CommandLine shared option instances across all App command trees, and its lazy ??= initialization is racy. When multiple App instances are constructed concurrently (as the parallel test classes in AspNetCore.CommandLine.Tests do), a command tree could end up holding a different option instance than the one its action later passes to parseResult.GetValue, which silently returns null. For schema export this meant the --output value was dropped and the schema was exported to the current directory with exit code 0, surfacing as an intermittent snapshot failure in SchemaExportCommandTests on CI.

Each command now creates its own option instances and captures them in its action closure, removing all shared symbol state between command trees. The Opt<T> helper is deleted.

The Nitro CLI has its own copy of the same pattern; that is left for a follow-up.

Test plan

  • dotnet test on AspNetCore.CommandLine.Tests (net10.0): all 12 tests pass.
  • Race mechanism was reproduced in isolation against System.CommandLine 2.0.5 (silently lost --output in 36/5000 simulated process starts with the singleton pattern); the per-instance design removes the shared state that race requires.

Copilot AI review requested due to automatic review settings June 11, 2026 07:21

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR removes shared System.CommandLine symbol singletons from AspNetCore.CommandLine to avoid racy, cross-command-tree option reuse when multiple CLI App instances are constructed concurrently (per System.CommandLine maintainers’ non-thread-safe guidance).

Changes:

  • Deleted the Opt<TOption> static singleton helper that shared Option instances across command trees.
  • Updated PrintCommand and ExportCommand to create per-command Option instances and capture them in the action closure used for parseResult.GetValue(...).

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
src/HotChocolate/AspNetCore/src/AspNetCore.CommandLine/Helpers/Opt.cs Removes the singleton option factory that could introduce cross-tree races.
src/HotChocolate/AspNetCore/src/AspNetCore.CommandLine/Command/PrintCommand.cs Uses a per-command SchemaNameOption instance and retrieves values via the captured instance.
src/HotChocolate/AspNetCore/src/AspNetCore.CommandLine/Command/ExportCommand.cs Uses per-command OutputOption / SchemaNameOption / SemanticNonNullOption instances and retrieves values via captured instances.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@glen-84
glen-84 merged commit 50d702e into main Jun 11, 2026
277 of 279 checks passed
@glen-84
glen-84 deleted the gai/per-instance-command-line-options branch June 11, 2026 07:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.