fix: improve thread-safe and atomic writing#204
Conversation
…riments and callbacks in GrowthBook, move fireSubscriptions to Util class
… ConcurrentHashMap and CopyOnWriteArrayList for assigned experiments and callbacks in GrowthBook, move fireSubscriptions to Util class
… and InMemoryStickyBucketServiceImpl
madhuchavva
left a comment
There was a problem hiding this comment.
Public getter compatibility issue found.
| //@Getter | ||
| @Getter | ||
| private Map<String, Feature<?>> parsedFeatures = new HashMap<>(); | ||
| private final AtomicReference<Map<String, Feature<?>>> parsedFeatures = new AtomicReference<>(new HashMap<>()); |
There was a problem hiding this comment.
[P1] Preserve the existing public getter return types
These atomics are exposed through Lombok-generated getters, so existing public APIs change shape. For example, getParsedFeatures() becomes AtomicReference<Map<String, Feature<?>>> instead of Map<String, Feature<?>>, getParsedSavedGroups() becomes AtomicReference<JsonObject>, and getInitialized() becomes AtomicBoolean. That is a source/binary compatibility break for a thread-safety fix. Please keep the atomic fields private and add explicit getters that preserve the existing return types.
madhuchavva
left a comment
There was a problem hiding this comment.
Requesting changes for the public API compatibility issue noted inline.
Summary
This PR addresses thread-safety issues across several core components of the GrowthBook Java SDK.
Changes
GBFeaturesRepositoryAtomicBoolean,AtomicLong,AtomicReferencewhere appropriateArrayListfor refresh callbacks toCopyOnWriteArrayListrefreshStrategyandsseRequestasvolatilesseHttpClientandsseEventSourceplain fields withAtomicReferenceFileCachingManagerImplFiles.move(ATOMIC_MOVE)pattern to prevent partial reads during concurrent accesssaveContentandloadCachewith class-level synchronizationGrowthBookHashMapwithConcurrentHashMapfor assigned experiments mapArrayListwithCopyOnWriteArrayListfor callbackscallbacksandevaluationContextasvolatileGrowthBookClientrepositoryfield asvolatileinitialize()withsynchronizedblock to prevent double-initializationHashMap/ArrayListwithConcurrentHashMap/CopyOnWriteArrayListglobalContextasvolatileGrowthBookUtilsfireSubscriptionsfrom bothGrowthBookandGrowthBookClientinto a shared utility method to eliminate duplicationInMemoryStickyBucketServiceImplNativeJavaGbFeatureRepositoryGbCacheManagerinitialization order