From 2cc80597527a233f05137514fc4ca2846f1c41a3 Mon Sep 17 00:00:00 2001 From: Abdullah Alaqeel Date: Wed, 29 Jul 2026 16:10:30 +0300 Subject: [PATCH] fix(builder): make model declaration check-and-create atomic MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit AddModelDeclarationIfDoesntExist checked for an existing class and created a new one in separate non-atomic steps. When two parallel threads both saw 'doesn't exist', both created class stubs — one with properties, one empty. The empty stub could be returned as a type definition, causing missing or different model files between runs (idempotency test failures). Fix: lock on the classLifecycles entry (keyed by namespace + class name) for the entire check-and-create. The lock is reentrant (recursive parent-schema and self-reference calls on the same thread don't deadlock) and per-class-name (different classes don't contend). --- src/Kiota.Builder/KiotaBuilder.cs | 14 +++- .../Kiota.Builder.Tests/KiotaBuilderTests.cs | 77 +++++++++++++++++++ 2 files changed, 90 insertions(+), 1 deletion(-) diff --git a/src/Kiota.Builder/KiotaBuilder.cs b/src/Kiota.Builder/KiotaBuilder.cs index 1045c042e0..6c4cd4d8e2 100644 --- a/src/Kiota.Builder/KiotaBuilder.cs +++ b/src/Kiota.Builder/KiotaBuilder.cs @@ -2123,11 +2123,15 @@ private CodeNamespace GetSearchNamespace(OpenApiUrlTreeNode currentNode, CodeNam return currentNamespace; } private ConcurrentDictionary classLifecycles = new(StringComparer.OrdinalIgnoreCase); + // Test-only hook for coordinating callers between the initial lookup and class publication. + internal Action? BeforeModelDeclarationCreation { get; set; } + internal Action? BeforeModelClassPublication { get; set; } private static readonly ThreadLocal> schemasBeingProcessedForDiscriminators = new(() => new(StringComparer.OrdinalIgnoreCase)); private CodeElement AddModelDeclarationIfDoesntExist(OpenApiUrlTreeNode currentNode, OpenApiOperation? currentOperation, IOpenApiSchema schema, string declarationName, CodeNamespace currentNamespace, CodeClass? inheritsFrom = null) { if (GetExistingDeclaration(currentNamespace, currentNode, declarationName) is not CodeElement existingDeclaration) // we can find it in the components { + BeforeModelDeclarationCreation?.Invoke(); if (AddEnumDeclaration(currentNode, schema, declarationName, currentNamespace) is CodeEnum enumDeclaration) return enumDeclaration; @@ -2266,8 +2270,16 @@ private CodeClass AddModelClass(OpenApiUrlTreeNode currentNode, IOpenApiSchema s var includeAdditionalDataProperties = config.IncludeAdditionalData && (schema.AdditionalPropertiesAllowed || schema.AdditionalProperties is not null); AddSerializationMembers(newClassStub, includeAdditionalDataProperties, config.UsesBackingStore, static s => s); - var newClass = currentNamespace.AddClass(newClassStub).First(); var lifecycle = classLifecycles.GetOrAdd(currentNamespace.Name + "." + declarationName, static n => new()); + CodeClass newClass; + lock (lifecycle) + { + // Another thread may have added this class while we were creating the stub. + if (GetExistingDeclaration(currentNamespace, currentNode, declarationName) is CodeClass existingClass) + return existingClass; + BeforeModelClassPublication?.Invoke(); + newClass = currentNamespace.AddClass(newClassStub).First(); + } if (!lifecycle.IsPropertiesBuilt() && !lifecycle.IsPropertiesBuildingInProgress()) { try diff --git a/tests/Kiota.Builder.Tests/KiotaBuilderTests.cs b/tests/Kiota.Builder.Tests/KiotaBuilderTests.cs index 16a52c8fa8..21d0331955 100644 --- a/tests/Kiota.Builder.Tests/KiotaBuilderTests.cs +++ b/tests/Kiota.Builder.Tests/KiotaBuilderTests.cs @@ -63,6 +63,83 @@ public async Task CreateOpenApiDocumentWithResultAsync_ReturnsDiagnostics() Assert.Equal(OpenApiSpecVersion.OpenApi3_0, diagnostics.SpecificationVersion); } [Fact] + public async Task GeneratesSharedModelWhenConcurrentCallersPassInitialLookup() + { + var specPath = Path.GetTempFileName(); + _tempFiles.Add(specPath); + var outputPath = Path.Combine(Path.GetTempPath(), $"kiota-shared-model-{Guid.NewGuid():N}"); + await File.WriteAllTextAsync(specPath, """ + openapi: 3.0.3 + info: + title: Shared model API + version: 1.0.0 + servers: + - url: https://example.com + paths: + /first: + get: + operationId: getFirst + responses: + '200': + description: User + content: + application/json: + schema: + $ref: '#/components/schemas/User' + /second: + get: + operationId: getSecond + responses: + '200': + description: User + content: + application/json: + schema: + $ref: '#/components/schemas/User' + components: + schemas: + User: + type: object + properties: + displayName: + type: string + """, TestContext.Current.CancellationToken); + + using var barrier = new Barrier(2); + var arrivals = 0; + var publicationAttempts = 0; + var builder = new KiotaBuilder(NullLogger.Instance, new GenerationConfiguration + { + ClientClassName = "ApiClient", + Language = GenerationLanguage.CSharp, + OpenAPIFilePath = specPath, + OutputPath = outputPath, + NoWorkspace = true, + MaxDegreeOfParallelism = 2, + }, _httpClient); + builder.BeforeModelDeclarationCreation = () => + { + if (Interlocked.Increment(ref arrivals) <= 2) + Assert.True(barrier.SignalAndWait(TimeSpan.FromSeconds(10))); + }; + builder.BeforeModelClassPublication = () => Interlocked.Increment(ref publicationAttempts); + try + { + await builder.GenerateClientAsync(TestContext.Current.CancellationToken); + + Assert.Equal(2, arrivals); + Assert.Equal(1, publicationAttempts); + Assert.Single(Directory.EnumerateFiles(outputPath, "User.cs", SearchOption.AllDirectories)); + } + finally + { + builder.BeforeModelDeclarationCreation = null; + builder.BeforeModelClassPublication = null; + if (Directory.Exists(outputPath)) + Directory.Delete(outputPath, true); + } + } + [Fact] public async Task SupportsExternalReferences() { var tempFilePathReferee = Path.GetTempFileName();