Skip to content

Conversation

@vazarkevych
Copy link
Collaborator

  • Introduced NSRecursiveLock plus getEvalContext, copyEvalContextLocked, and updateEvalContext(with:) to guard GrowthBookSDK.evalContext against concurrent access.
  • getFeatureValue now grabs a locked copy of the evaluation context, runs the FeatureEvaluator, and merges the results back, eliminating race conditions when called from multiple threads.
  • The same lock-aware flow now covers evalFeature, run, setAttributes, appendAttributes, setForcedFeatures, setForcedVariations, etc., so every code path that mutates shared state stays thread-safe.

Copy link
Contributor

@madhuchavva madhuchavva left a comment

Choose a reason for hiding this comment

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

The approach here is to:

  1. Lock → Copy → Unlock
  2. Evaluate (unlocked)
  3. Lock → Merge back → Unlock

I think this is a bit complex way to manage the state and If two threads evaluate simultaneously, the second thread's updateEvalContext will overwrite the first thread's changes??

can we have a simple approach of using single lock around entire evaluation? that way it serializes the entire evaluation? here's an eg:

private let evaluationLock = NSLock() // Regular lock is fine

public func getFeatureValue(feature id: String, default defaultValue: JSON) -> JSON {
    evaluationLock.lock()
    defer { evaluationLock.unlock() }
    
    let result = FeatureEvaluator(context: evalContext, featureKey: id).evaluateFeature()
    return result.value ?? defaultValue
}

public func evalFeature(id: String) -> FeatureResult {
    evaluationLock.lock()
    defer { evaluationLock.unlock() }
    
    return FeatureEvaluator(context: evalContext, featureKey: id).evaluateFeature()
}

public func run(experiment: Experiment) -> ExperimentResult {
    evaluationLock.lock()
    defer { evaluationLock.unlock() }
    
    return ExperimentEvaluator().evaluateExperiment(context: evalContext, experiment: experiment)
}

@vazarkevych vazarkevych merged commit 6e2a6eb into growthbook:main Nov 24, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants