Skip to content

Fix TypeFactory.CreateCSharpType re-entrant crash on self-referencing models#10032

Open
haiyuazhang wants to merge 1 commit intomainfrom
fix/typefactory-reentrant-cache-crash
Open

Fix TypeFactory.CreateCSharpType re-entrant crash on self-referencing models#10032
haiyuazhang wants to merge 1 commit intomainfrom
fix/typefactory-reentrant-cache-crash

Conversation

@haiyuazhang
Copy link
Member

Summary

Fixes #10031

Self-referencing models (e.g. QueryFilter with and: QueryFilter[]) cause TypeFactory.CreateCSharpType to crash with System.ArgumentException: An item with the same key has already been added. This is because recursive property resolution triggers re-entrant calls that Dictionary.Add() cannot handle.

Change

One-line fix in TypeFactory.cs: replace TypeCache.Add(inputType, type) with TypeCache[inputType] = type.

The indexer assignment is idempotent — when the inner (re-entrant) call adds the cache entry first, the outer call simply overwrites with the identical value.

Call Flow (before fix)

CreateCSharpType(QueryFilter)          // outer — cache miss
  → CreateCSharpTypeCore
    → CreateModel → BuildProperties
      → CreateCSharpType(QueryFilter[])
        → CreateCSharpType(QueryFilter)  // inner — cache miss (outer hasn't added yet)
          → resolves → TypeCache.Add(QueryFilter, ...) ✅
      → outer resumes → TypeCache.Add(QueryFilter, ...) 💥 duplicate key

Regression Testing

All existing tests pass with zero regressions:

Test Suite Passed Failed
Generator.Tests 1,274 0
ClientModel.Tests 1,216 0
Input.Tests 88 0
Local.Tests (Sample) 37 0
Spector.Tests 15 (843 skipped by design) 0
Regen check (63 specs) 63/63 0
Git diff after regen Zero diff

🤖 This PR was authored by GitHub Copilot

… models

Self-referencing models (e.g. QueryFilter with 'and: QueryFilter[]') cause
CreateCSharpTypeCore to recursively call CreateCSharpType for the same
InputType. The inner call adds the type to TypeCache via Add(), then the
outer call's Add() throws 'An item with the same key has already been added'.

Replace Dictionary.Add() with indexer assignment which is idempotent — both
calls resolve to the same type, so the second write is a safe no-op.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@microsoft-github-policy-service microsoft-github-policy-service bot added the emitter:client:csharp Issue for the C# client emitter: @typespec/http-client-csharp label Mar 15, 2026
@pkg-pr-new
Copy link

pkg-pr-new bot commented Mar 15, 2026

Open in StackBlitz

npm i https://pkg.pr.new/@typespec/http-client-csharp@10032

commit: 27bcc5d

@github-actions
Copy link
Contributor

No changes needing a change description found.

@ArcturusZhang
Copy link
Member

should not we refine the flow so that every time we have updated the cache first?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

emitter:client:csharp Issue for the C# client emitter: @typespec/http-client-csharp

Projects

None yet

Development

Successfully merging this pull request may close these issues.

TypeFactory.CreateCSharpType crashes on self-referencing models (re-entrant Dictionary.Add)

2 participants