Skip to content

fix(extension): eliminate race-prone global registries in common/extension#3264

Open
xxs588 wants to merge 13 commits intoapache:developfrom
xxs588:fix/data-races-extension-3247-clean
Open

fix(extension): eliminate race-prone global registries in common/extension#3264
xxs588 wants to merge 13 commits intoapache:developfrom
xxs588:fix/data-races-extension-3247-clean

Conversation

@xxs588
Copy link

@xxs588 xxs588 commented Mar 17, 2026

Description

Fixes #3247

This PR addresses Category-1 race conditions in common/extension by replacing unsafe global maps with a thread-safe generic Registry[T] and migrating extension registries in atomic commits.

Key changes:

  • add common/extension/registry_type.go + registry_type_test.go
  • migrate extension registry access paths to Register/Get/MustGet/Unregister/Snapshot/Names
  • harden non-map shared state (registry_directory, customizers, shutdown callbacks, service-name mapping)

Local verification:

  • go test -race ./common/extension/...

Checklist

  • I confirm the target branch is develop
  • Code has passed local testing
  • I have added tests that prove my fix is effective or that my feature works

@xxs588 xxs588 changed the base branch from main to develop March 17, 2026 15:09
@xxs588 xxs588 marked this pull request as draft March 17, 2026 15:21
@xxs588 xxs588 force-pushed the fix/data-races-extension-3247-clean branch from 7a46ab4 to 9eeded4 Compare March 17, 2026 15:32
@xxs588 xxs588 marked this pull request as ready for review March 17, 2026 15:44
@codecov-commenter
Copy link

codecov-commenter commented Mar 17, 2026

Codecov Report

❌ Patch coverage is 55.33333% with 67 lines in your changes missing coverage. Please review.
✅ Project coverage is 48.33%. Comparing base (60d1c2a) to head (bc5c9e0).
⚠️ Report is 757 commits behind head on develop.

Files with missing lines Patch % Lines
common/extension/auth.go 0.00% 7 Missing ⚠️
common/extension/configurator.go 0.00% 5 Missing ⚠️
common/extension/cluster.go 0.00% 4 Missing ⚠️
common/extension/config_center_factory.go 0.00% 4 Missing ⚠️
common/extension/config_reader.go 0.00% 4 Missing ⚠️
common/extension/metadata_report_factory.go 0.00% 4 Missing ⚠️
common/extension/proxy_factory.go 0.00% 4 Missing ⚠️
common/extension/registry.go 0.00% 4 Missing ⚠️
common/extension/tps_limit.go 0.00% 4 Missing ⚠️
common/extension/config_center.go 0.00% 3 Missing ⚠️
... and 11 more
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #3264      +/-   ##
===========================================
+ Coverage    46.76%   48.33%   +1.56%     
===========================================
  Files          295      467     +172     
  Lines        17172    34081   +16909     
===========================================
+ Hits          8031    16474    +8443     
- Misses        8287    16274    +7987     
- Partials       854     1333     +479     

☔ 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.

@xxs588 xxs588 marked this pull request as draft March 17, 2026 16:35
@xxs588 xxs588 marked this pull request as ready for review March 18, 2026 02:11
@Alanxtl Alanxtl added ☢️ Bug 3.3.2 version 3.3.2 labels Mar 18, 2026
@Alanxtl
Copy link
Contributor

Alanxtl commented Mar 18, 2026

@zbchi check this

Copy link
Contributor

@zbchi zbchi left a comment

Choose a reason for hiding this comment

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

lgtm

)

var configCenterFactories = make(map[string]func() config_center.DynamicConfigurationFactory)
var configCenterFactories = NewRegistry[func() config_center.DynamicConfigurationFactory]("config center")
Copy link
Contributor

Choose a reason for hiding this comment

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

这个也是"config center"吗

Copy link
Author

Choose a reason for hiding this comment

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

ohno,我马上修改,感谢

Copy link
Author

Choose a reason for hiding this comment

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

已按建议修正:将 config_center_factory.go 中的 registry 名称从 config center 调整为 config center factory,避免语义歧义。

对应提交:bc5c9e01
本地验证:make fmt, make lint, go test -race ./common/extension/... 均通过。谢谢指正。

Align registry name with config center factory semantics for clearer diagnostics in extension registration lookup.

Fixes: apache#3247
@sonarqubecloud
Copy link

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

Labels

3.3.2 version 3.3.2 ☢️ Bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Systematic data races: 20+ extension global maps unprotected, wrong lock in SetProviderService, RouterChain.Route lockless

4 participants