fix: thread-safe cache writes and feature update handling #114
Open
vazarkevych wants to merge 1 commit into
Open
fix: thread-safe cache writes and feature update handling #114vazarkevych wants to merge 1 commit into
vazarkevych wants to merge 1 commit into
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.
Problem
Several race conditions existed in cache and feature update handling:
InMemoryFeatureCachehad no locking — concurrent reads/writes could corrupt cache entriesFeatureRepository.load_featuresandload_features_asynchad no double-checked locking — multiple threads/coroutines could trigger redundant HTTP fetches simultaneously_feature_update_callbackslist was mutated and iterated without a lock — concurrentadd/remove/notifycould raiseRuntimeError: list changed size during iteration_sticky_bucket_cache_lockwas a boolean flag instead of a real lock — the spin-loop was not thread-safe and silently returned{}when the "lock" was heldFeatureCache.get_current_statereturned a mutable reference tosavedGroupsinstead of a copyGrowthBook.load_features_asynccalledsave_in_cachewith wrong cache key (client_keyinstead ofapi_host::client_key), making the cached value unreachable_features_event_handlerhad the same incorrect cache keyChanges
InMemoryFeatureCache: addedthreading.Locktoget,set,clearFeatureRepository: added_load_lockand_async_load_lockwith double-checked locking pattern inload_featuresandload_features_async_feature_update_callbacks: protectedadd/removewith_refresh_lock;_notifycopies the list under the lock and iterates outside to prevent deadlocks from slow callbacks_sticky_bucket_cache_lock: replaced boolean spin-lock withasyncio.Lock(); simplified_refresh_sticky_bucketsFeatureCache.get_current_state: returnsdict()copy ofsavedGroupsGrowthBook.load_features_async: removed redundantsave_in_cachecall (already handled byFeatureRepository)_features_event_handler: fixed cache key toapi_host::client_key; changedreturn Nonetoreturn