Fix circuit breaker probe lockup, cache key collision, evaluators leak, and HOCON list corruption#107
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
CircuitBreakerProvider: half-open probe slot permanently stuck on application errors
Application errors (FlagNotFound, TypeMismatch) in half-open state did not call
recordSuccessorrecordFailure, leaving the probe slot locked. All subsequent requests were rejected forever. Now treats application errors as success for circuit purposes since the provider is reachable.CachingProvider: cache key hash collision serves wrong flag values across users
CacheKeyused a 32-bitcontextHash: Int. Two different evaluation contexts could collide, causing user A to receive user B's cached flag value. Replaced withcontextFingerprint: Stringusing stable sorted serialization of context attributes.CachingProvider: unbounded evaluators map leaks memory
The
evaluators: ConcurrentHashMapside-channel had no eviction — entries accumulated for every unique cache key over the provider's lifetime. Changedevaluators.put()toevaluators.remove()in the Lookup callback so entries are cleaned up after cache population.HoconProvider: LIST config values become list of nulls
configValueToSdkValuecalled.asObject()on each list element, which returns null for scalar values. A config likeallowed-regions = ["us", "eu"]produced[null, null]. Removed the.asObject()call.Event bridge: getOrThrowFiberFailure can crash Java SDK event thread
All event bridge handlers used
getOrThrowFiberFailure()which could throw on defects (e.g., Hub shutdown), killing the Java SDK's internal event dispatch thread. Replaced withgetOrElseto absorb unexpected failures.Test assertions: overly broad
result.isLeftchecksBehaviorControlsSpec:setErrorMode(ProviderNotReady)now asserts the specific error typeEvaluationTimeoutSpec: per-call timeout tests now assertProviderErrortype