Fix startup NPE when a weakly dependent registry is unavailable (check=false)#16356
Fix startup NPE when a weakly dependent registry is unavailable (check=false)#16356AryamannSingh7 wants to merge 1 commit into
Conversation
…check=false When a registry cannot be created and check=false (e.g. a weakly dependent ZooKeeper is down at startup), AbstractRegistryFactory returns a null registry which RegistryFactoryWrapper still wraps in a ListenerRegistryWrapper. The wrapper already guards register/unregister/subscribe against a null delegate, but getUrl/isAvailable/destroy/unsubscribe/isServiceDiscovery/lookup did not, and RegistryDirectory#subscribe dereferences registry.getUrl() unconditionally to build a metrics label (added in apache#12582), causing an NPE since 3.2.5. Complete the null-safety in ListenerRegistryWrapper and guard the metrics label computation in RegistryDirectory#subscribe so a service can still start when a non-critical registry is unavailable. Fixes apache#16178
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## 3.3 #16356 +/- ##
============================================
+ Coverage 60.81% 60.85% +0.03%
- Complexity 30 11762 +11732
============================================
Files 1953 1953
Lines 89213 89217 +4
Branches 13460 13460
============================================
+ Hits 54254 54290 +36
+ Misses 29374 29349 -25
+ Partials 5585 5578 -7
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
LI123456mo
left a comment
There was a problem hiding this comment.
Reviewed the changes. The fix correctly guards all unprotected methods in ListenerRegistryWrapper (getUrl, isAvailable, destroy, unsubscribe, isServiceDiscovery, lookup) against a null delegate, and the RegistryDirectory#subscribe change safely extracts registry.getUrl() into a local variable before use. The test reproduces the exact NPE scenario.
@zrlw check it aslo
LGTM.
What is the purpose of the change?
Fixes #16178.
When a service subscribes to multiple registries and one of them is weakly dependent (
check="false") and unavailable at startup, the application fails to start with:This is a regression: Dubbo
3.2.0–3.2.4start successfully,3.2.5+ fail (also reproduced on3.3.x).Root cause
Under
check=false, when a registry cannot be created (the registry is down),AbstractRegistryFactory#getRegistryswallows the exception and returnsnull.RegistryFactoryWrapperstill wraps thisnullinto aListenerRegistryWrapper, so a null delegate is an expected state —ListenerRegistryWrapperalready guardsregister/unregister/subscribewithif (registry != null).However:
getUrl/isAvailable/destroy/unsubscribe/isServiceDiscovery/lookupwere not guarded.RegistryDirectory#subscribecomputes a metrics cluster name viaregistry.getUrl().getParameter(...)unconditionally. That line was introduced in Support multi registries metrics key #12582 (released in3.2.5), which is exactly why the NPE appears from3.2.5onward.What changed
ListenerRegistryWrapper— complete the existing null-safety:getUrlreturnsnull,isAvailable/isServiceDiscoveryreturnfalse,destroy/unsubscribebecome no-ops, andlookupreturns an empty list when the delegate registry isnull.RegistryDirectory#subscribe— guard the metrics cluster-name computation against anullregistry URL (no cluster name is reported when the registry is unavailable).RegistryEvent#toSubscribeEventalready tolerates anullname.Verification
Added
ListenerRegistryWrapperTest#testNullRegistryIsTolerated, which reproduces the exact NPE without the fix and passes with it.:dubbo-registry-apitests andspotless:checkpass locally (JDK 21).Checklist