Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix memory leak for multiple runs in the same process #2987

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

iridakos
Copy link

@iridakos iridakos commented Dec 11, 2022

As reported on #2767 by @agis, there's a memory leak in RSpec when having multiple runs in the same process.

Causes

After a lot of investigation, I've managed to locate the leak's causes and those caused by the rspec-core's gem are resolved with the introduced changes of this PR.

RSpec::Core::World - @filtered_examples

The world's instance variable @filtered_examples' keys are classes of RSpec::Core::ExampleGroup generated for a run's example groups (a.k.a. RSpec::ExamplesGroups::Foo).

Since the instance variable is not being cleared between runs, it keeps references to these classes (along with their "expensive" state - each group class contains all of its examples along with their reporter/loader/formatter etc).

Note: even though the RSpec::ExampleGroups.remove_all_constants does remove the constants, the references above keep them alive.

RSpec::Core::AnonymousExampleGroup

Examples that are added to the RSpec::Core::AnonymousExampleGroup upon their initialization (ex. RSpec::Core::SuiteHookContext) don't currently get cleared between runs.

RSpec::Core::SharedExampleGroup::Registry

The RSpec::Core::SharedExampleGroup::Registry maintains references to RSpec::Core::ExampleGroup classes that were generated by previous runs (as keys in the @shared_example_groups hash) preventing them from being garbage collected.

Benchmarking

Introducing the changes of this PR to the reproduction script made by @agis confirmed the fix.

In the previous state, the memory keeps growing and on the 10.000th iteration has reached ~800Mb.
With this PR's fixes, the memory reaches a plateau pretty early and on the 10.000th iteration is at ~39Mb.

External causes

Besides rspec-core, I found other "external" causes for memory leaks depending on the codebase's used libraries.
I'll try to open the proper PRs for those as well. Until then, find below some info for each of them along with workarounds in case someone finds them helpful.

rspec-mocks & rspec-rails

The RSpec::Mocks::Configuration also keeps references to RSpec::Core::ExampleGroup classes.

The following rspec-rails section

      def self.initialize_activerecord_configuration(config)
        config.before :suite do
          # This allows dynamic columns etc to be used on ActiveRecord models when creating instance_doubles
          if defined?(ActiveRecord) && defined?(ActiveRecord::Base) && defined?(::RSpec::Mocks) && (::RSpec::Mocks.respond_to?(:configuration))
            ::RSpec::Mocks.configuration.when_declaring_verifying_double do |possible_model|
              target = possible_model.target

              if Class === target && ActiveRecord::Base > target && !target.abstract_class?
                target.define_attribute_methods
              end
            end
          end
        end
      end

registers a before suite hook in RSpec's configuration which in turn alter's RSpec::Mocks::Configuration

      def before_verifying_doubles(&block)
        verifying_double_callbacks << block
      end
      alias :when_declaring_verifying_double :before_verifying_doubles

I believe that some of these blocks are being defined through RSpec::Core::ExampleGroup class instances (when their SuiteHookContext examples execute) and since they live forever in the RSpec::Mocks::Configuration instance, they keep the references to their RSpec::Core::ExampleGroup alive forever.

Workaround (monkey patch):

module RSpec
  module Mocks
    def self.reset
      @configuration = nil
    end
  end

  module Core
    class World
      alias_method :original_reset, :reset

      def reset
        original_reset
        RSpec::Mocks.reset
      end
    end
  end
end

ActiveRecord

ActiveRecord also keeps RSpec::Core::ExampleGroup class instance references as keys in its @@already_loaded_fixtures class variable here:

      if run_in_transaction?
        if @@already_loaded_fixtures[self.class]
          @loaded_fixtures = @@already_loaded_fixtures[self.class]
        else
          @loaded_fixtures = load_fixtures(config)
          @@already_loaded_fixtures[self.class] = @loaded_fixtures
        end
        ...

Workaround (monkey patch):

module RSpec
  module Core
    class World
      alias_method :original_reset, :reset

      def reset
        original_reset
        
        ActiveRecord::TestFixtures.class_variable_get(:@@already_loaded_fixtures)&.delete_if do |klass, _|
          klass < RSpec::Core::ExampleGroup
        end
      end
    end
  end
end

ActiveSupport

ActiveSupport also keeps a record for each class for which it has loaded its hook here:

    def run_load_hooks(name, base = Object)
      @loaded[name] << base # <- base can be of RSpec::Core::ExampleGroup
      @load_hooks[name].each do |hook, options|
        execute_hook(name, base, options, hook)
      end
    end

The block above is being executed for bases that are RSpec::Core::ExampleGroup classes thus references to them live forever.

Workaround (monkey patch)

module RSpec
  module Core
    class World
      alias_method :original_reset, :reset

      def reset
        original_reset

        ActiveSupport.instance_variable_get(:@loaded)&.each do |_, bases|
          bases.delete_if do |base|
            base.is_a?(Class) and base < RSpec::Core::ExampleGroup
          end
        end
      end
    end
  end
end

Closes #2767

The `RSpec::ExampleGroups.remove_all_constants` will unset the
RSpec::Core::ExampleGroup subclass constants that where generated
during a run, though the `@filtered_examples` hash's keys still
reference them preventing them from being Garbage collected.
Examples that are added to the RSpec::Core::AnonymousExampleGroup upon
their initialization (ex. RSpec::Core::SuiteHookContext) don't currently
get cleared between runs.

See RSpec::Core::Example#initialize
The RSpec::Core::SharedExampleGroup maintains references to
RSpec::Core::ExampleGroup classes that were generated to previous
runs preventing them to be garbage collected.
Copy link
Member

@JonRowe JonRowe left a comment

Choose a reason for hiding this comment

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

👋 thanks for this, we would need specs for the changes and in particular for the shared example group to make sure we aren't accidentally clearing shared example definitions that are needed, I'm assuming your code will just remove the ones attached to now cleared specs but it'd be good to verify that none of the other ways are specifying them are cleared accidentally

@@ -180,6 +180,10 @@ def find(lookup_contexts, name)
shared_example_groups[:main][name]
end

def reset
shared_example_groups.delete_if { |k, _| k != :main }
end
Copy link
Member

Choose a reason for hiding this comment

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

We would need a spec for this, as its important child shared example groups are still usable, its intentional they are not cleared (but unintentional that references to deleted example groups are kept around)

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.

Ever-increasing memory with a custom runner and a single spec file
2 participants