Skip to content

Commit 7926242

Browse files
jnunemakerclaude
andcommitted
Fix thread-local leak on self-reference and circular dependency
Move begin/ensure to wrap the early return path so the thread-local evaluating set is always cleaned up, even when a circular dependency or self-reference triggers the early return. Added tests to verify. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent a217680 commit 7926242

2 files changed

Lines changed: 21 additions & 4 deletions

File tree

lib/flipper/expressions/feature_enabled.rb

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,11 @@ def self.call(feature_name, context:)
1313
# Track the current feature so A -> B -> A is caught
1414
added_current = evaluating.add?(current_feature)
1515

16-
# Circular dependency: return false to break the cycle
17-
return false if evaluating.include?(feature_name)
18-
19-
evaluating.add(feature_name)
2016
begin
17+
# Circular dependency: return false to break the cycle
18+
return false if evaluating.include?(feature_name)
19+
20+
evaluating.add(feature_name)
2121
actor = context[:actor]
2222
if actor
2323
Flipper.enabled?(feature_name, actor)

spec/flipper_integration_spec.rb

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -653,6 +653,23 @@
653653
evaluating = Thread.current[:flipper_evaluating_features]
654654
expect(evaluating.nil? || evaluating.empty?).to be(true)
655655
end
656+
657+
it "cleans up thread-local state after self-reference" do
658+
feature.enable Flipper.feature_enabled(:search)
659+
660+
feature.enabled?
661+
evaluating = Thread.current[:flipper_evaluating_features]
662+
expect(evaluating.nil? || evaluating.empty?).to be(true)
663+
end
664+
665+
it "cleans up thread-local state after circular dependency" do
666+
feature.enable Flipper.feature_enabled(:other)
667+
flipper[:other].enable Flipper.feature_enabled(:search)
668+
669+
feature.enabled?
670+
evaluating = Thread.current[:flipper_evaluating_features]
671+
expect(evaluating.nil? || evaluating.empty?).to be(true)
672+
end
656673
end
657674

658675
context "for FeatureDisabled" do

0 commit comments

Comments
 (0)