Skip to content

Fix shared options mutation in AbpRemoteServiceApiDescriptionProvider#25542

Merged
EngincanV merged 1 commit into
rel-10.4from
maliming/fix-25536
Jun 3, 2026
Merged

Fix shared options mutation in AbpRemoteServiceApiDescriptionProvider#25542
EngincanV merged 1 commit into
rel-10.4from
maliming/fix-25536

Conversation

@maliming

@maliming maliming commented Jun 3, 2026

Copy link
Copy Markdown
Member

Fixes #25536.

GetApiResponseTypes() mutated the singleton IOptions<AbpRemoteServiceApiDescriptionProviderOptions>.SupportedResponseTypes on every call, causing unbounded growth of ApiResponseFormats and concurrent List<>.Add races on /Abp/ServiceProxyScript. Each call now yields fresh ApiResponseType instances.

Copilot AI review requested due to automatic review settings June 3, 2026 07:33
@maliming maliming added this to the 10.4-patch milestone Jun 3, 2026

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

Fixes a concurrency/memory-growth bug in ABP’s MVC API explorer integration by ensuring AbpRemoteServiceApiDescriptionProvider.GetApiResponseTypes() no longer mutates the shared singleton options (IOptions<AbpRemoteServiceApiDescriptionProviderOptions>.Value.SupportedResponseTypes) during API description rebuilds.

Changes:

  • Update GetApiResponseTypes() to create and yield fresh ApiResponseType instances (including newly built ApiResponseFormats) instead of modifying the option templates.
  • Add test coverage to verify templates are not mutated across repeated calls and that concurrent executions do not throw.

Reviewed changes

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

File Description
framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/ApiExploring/AbpRemoteServiceApiDescriptionProvider.cs Avoids shared options mutation by cloning response type templates per call and building formats on the new instances.
framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/ApiExploring/AbpRemoteServiceApiDescriptionProvider_Tests.cs Adds regression tests for non-mutation, correct response type creation, and concurrency safety.

@codecov

codecov Bot commented Jun 3, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 11.42857% with 93 lines in your changes missing coverage. Please review.
✅ Project coverage is 49.41%. Comparing base (36546b2) to head (a860b00).
⚠️ Report is 440 commits behind head on rel-10.4.

Files with missing lines Patch % Lines
...ng/AbpRemoteServiceApiDescriptionProvider_Tests.cs 0.00% 93 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##           rel-10.4   #25542      +/-   ##
============================================
- Coverage     49.42%   49.41%   -0.02%     
============================================
  Files          3670     3671       +1     
  Lines        123599   123848     +249     
  Branches       9453     9466      +13     
============================================
+ Hits          61091    61201     +110     
- Misses        60672    60816     +144     
+ Partials       1836     1831       -5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@HamzaSallakh

Copy link
Copy Markdown
Contributor

Thank you for the quick fix. I was preparing a pull request for this, but you beat me to it 🙂 The solution is clever and clean. I really appreciate the fast response.

@EngincanV EngincanV merged commit a60153f into rel-10.4 Jun 3, 2026
3 of 5 checks passed
@EngincanV EngincanV deleted the maliming/fix-25536 branch June 3, 2026 08:20
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.

4 participants