From 7432c9226e67de1b9a334a097446ea8c12350dbe Mon Sep 17 00:00:00 2001 From: Edgars Beigarts Date: Thu, 14 Dec 2017 12:37:51 +0200 Subject: [PATCH] Use weak references in descendants tracker It allows anonymous subclasses to be garbage collected. --- activesupport/CHANGELOG.md | 5 ++ .../lib/active_support/descendants_tracker.rb | 54 +++++++++++++++++-- .../test/descendants_tracker_test_cases.rb | 9 ++++ 3 files changed, 64 insertions(+), 4 deletions(-) diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md index 03c9dfc546..0551f0781f 100644 --- a/activesupport/CHANGELOG.md +++ b/activesupport/CHANGELOG.md @@ -1,3 +1,8 @@ +* Use weak references in descendants tracker to allow anonymous subclasses to + be garbage collected. + + *Edgars Beigarts* + * Update `ActiveSupport::Notifications::Instrumenter#instrument` to make passing a block optional. This will let users use `ActiveSupport::Notifications` messaging features outside of diff --git a/activesupport/lib/active_support/descendants_tracker.rb b/activesupport/lib/active_support/descendants_tracker.rb index 05236d3162..2dca990712 100644 --- a/activesupport/lib/active_support/descendants_tracker.rb +++ b/activesupport/lib/active_support/descendants_tracker.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require "weakref" + module ActiveSupport # This module provides an internal implementation to track descendants # which is faster than iterating through ObjectSpace. @@ -8,7 +10,8 @@ module ActiveSupport class << self def direct_descendants(klass) - @@direct_descendants[klass] || [] + descendants = @@direct_descendants[klass] + descendants ? descendants.to_a : [] end def descendants(klass) @@ -34,15 +37,17 @@ module ActiveSupport # This is the only method that is not thread safe, but is only ever called # during the eager loading phase. def store_inherited(klass, descendant) - (@@direct_descendants[klass] ||= []) << descendant + (@@direct_descendants[klass] ||= DescendantsArray.new) << descendant end private def accumulate_descendants(klass, acc) if direct_descendants = @@direct_descendants[klass] - acc.concat(direct_descendants) - direct_descendants.each { |direct_descendant| accumulate_descendants(direct_descendant, acc) } + direct_descendants.each do |direct_descendant| + acc << direct_descendant + accumulate_descendants(direct_descendant, acc) + end end end end @@ -59,5 +64,46 @@ module ActiveSupport def descendants DescendantsTracker.descendants(self) end + + # DescendantsArray is an array that contains weak references to classes. + class DescendantsArray # :nodoc: + include Enumerable + + def initialize + @refs = [] + end + + def initialize_copy(orig) + @refs = @refs.dup + end + + def <<(klass) + cleanup! + @refs << WeakRef.new(klass) + end + + def each + @refs.each do |ref| + yield ref.__getobj__ + rescue WeakRef::RefError + end + end + + def refs_size + @refs.size + end + + def cleanup! + @refs.delete_if { |ref| !ref.weakref_alive? } + end + + def reject! + @refs.reject! do |ref| + yield ref.__getobj__ + rescue WeakRef::RefError + true + end + end + end end end diff --git a/activesupport/test/descendants_tracker_test_cases.rb b/activesupport/test/descendants_tracker_test_cases.rb index 2c94c3c56c..f8752688d2 100644 --- a/activesupport/test/descendants_tracker_test_cases.rb +++ b/activesupport/test/descendants_tracker_test_cases.rb @@ -27,6 +27,15 @@ module DescendantsTrackerTestCases assert_equal_sets [], Child2.descendants end + def test_descendants_with_garbage_collected_classes + 1.times do + child_klass = Class.new(Parent) + assert_equal_sets [Child1, Grandchild1, Grandchild2, Child2, child_klass], Parent.descendants + end + GC.start + assert_equal_sets [Child1, Grandchild1, Grandchild2, Child2], Parent.descendants + end + def test_direct_descendants assert_equal_sets [Child1, Child2], Parent.direct_descendants assert_equal_sets [Grandchild1, Grandchild2], Child1.direct_descendants