Merge branch 'feature/MSP-11130/metasploit-framework-spec-constants' …

…into feature/MSP-11147/thread-leak-detection

MSP-11147

Merge to get framework instance cleanup, which should clean up a lot of
thread leaks too.

Conflicts:
	Rakefile
	lib/metasploit/framework/spec.rb
	spec/spec_helper.rb

Rakefile

@@ -10,5 +10,6 @@ require 'metasploit/framework/spec/untested_payloads'
 Metasploit::Framework::Require.optionally_active_record_railtie
 
 Metasploit::Framework::Application.load_tasks
+Metasploit::Framework::Spec::Constants.define_task
 Metasploit::Framework::Spec::Threads::Suite.define_task
 Metasploit::Framework::Spec::UntestedPayloads.define_task

lib/metasploit/framework/spec.rb

@@ -1,5 +1,6 @@
 module Metasploit::Framework::Spec
   extend ActiveSupport::Autoload
 
+  autoload :Constants
   autoload :Threads
 end

lib/metasploit/framework/spec/constants.rb

@@ -0,0 +1,99 @@
+require 'msf/core/modules'
+
+# Monitor constants created by module loading to ensure that the loads in one example don't interfere with the
+# assertions in another example.
+module Metasploit::Framework::Spec::Constants
+  extend ActiveSupport::Autoload
+
+  autoload :Each
+  autoload :Suite
+
+  #
+  # CONSTANTS
+  #
+
+  # Regex parsing loaded module constants
+  LOADED_MODULE_CHILD_CONSTANT_REGEXP = /^Mod(?<unpacked_full_name>[0-9a-f]+)$/
+  # The parent namespace child_constant_name that can have children added when loading modules.
+  PARENT_CONSTANT = Msf::Modules
+  # Constant names under {PARENT_CONSTANT} that can persist between specs because they are part of the loader library
+  # and not dynamically loaded code
+  PERSISTENT_CHILD_CONSTANT_NAMES = %w{
+    Error
+    Loader
+    MetasploitClassCompatibilityError
+    Namespace
+    VersionCompatibilityError
+  }.map(&:to_sym)
+
+  # Cleans child constants from {PARENT_CONSTANT}.
+  #
+  # @return [true] if there were leaked constants that were cleaned.
+  # @return [false] if there were no leaked constants.
+  # @see each
+  def self.clean
+    count = each do |child_name|
+      PARENT_CONSTANT.send(:remove_const, child_name)
+    end
+
+    count != 0
+  end
+
+  # Adds actions to `spec` task so that `rake spec` fails if any of the following:
+  #
+  # # `log/leaked-constants.log` exists after printing out the leaked constants.
+  # # {Each.configured!} is unnecessary in `spec/spec_helper.rb` and should be removed.
+  #
+  # @return [void]
+  def self.define_task
+    Suite.define_task
+    # After Suite as Suite will kill for leaks before Each say it cleaned no leaks in case there are leaks in an
+    # `after(:all)` that {Each} won't catch in its `after(:each)` checks.
+    Each.define_task
+  end
+
+  # Yields each child_constant_name under {PARENT_CONSTANT}.
+  #
+  # @yield [child_name]
+  # @yieldparam child_name [Symbol] name of child_constant_name relative to {PARENT_CONSTANT}.
+  # @yieldreturn [void]
+  # @return [Integer] count
+  def self.each
+    inherit = false
+    count = 0
+
+    child_constant_names = PARENT_CONSTANT.constants(inherit)
+
+    child_constant_names.each do |child_constant_name|
+      unless PERSISTENT_CHILD_CONSTANT_NAMES.include? child_constant_name
+        count += 1
+        yield child_constant_name
+      end
+    end
+
+    count
+  end
+
+  # The module full name for `child_constant_name`
+  #
+  # @param child_constant_name [String] the name of a child constant_name under {PARENT_CONSTANT}.
+  # @return [String] full module name used to load `child_constant_name`.
+  # @return [nil] if `child_constant_name` does not correspond to a loaded module.
+  def self.full_name(child_constant_name)
+    full_name = nil
+
+    match = LOADED_MODULE_CHILD_CONSTANT_REGEXP.match(child_constant_name)
+
+    if match
+      potential_full_name = [match[:unpacked_full_name]].pack('H*')
+
+      module_type, _reference_name = potential_full_name.split('/', 2)
+
+      if Msf::MODULE_TYPES.include? module_type
+        full_name = potential_full_name
+      end
+    end
+
+    full_name
+  end
+end

lib/metasploit/framework/spec/constants/each.rb

@@ -0,0 +1,119 @@
+# @note This should only temporarily be used in `spec/spec_helper.rb` when
+#   `Metasploit::Framework::Spec::Constants::Suite.configure!` detects a leak.  Permanently having
+#   `Metasploit::Framework::Spec::Constants::Each.configure!` can lead to false positives when modules are purposely
+#   loaded in a `before(:all)` and cleaned up in a `after(:all)`.
+#
+# Fails example if it leaks module loading constants.
+module Metasploit::Framework::Spec::Constants::Each
+  #
+  # CONSTANTS
+  #
+
+  LOG_PATHNAME = Pathname.new('log/metasploit/framework/spec/constants/each.log')
+
+  #
+  # Module Methods
+  #
+
+  class << self
+    attr_accessor :leaks_cleaned
+  end
+
+  # Is {Metasploit::Framework::Spec::Constants::Each.configure!} still necessary or should it be removed?
+  #
+  # @return [true] if {configure!}'s `before(:each)` cleaned up leaked constants
+  # @return [false] otherwise
+  def self.leaks_cleaned?
+    !!@leaks_cleaned
+  end
+
+  # Configures after(:each) callback for RSpe to fail example if leaked constants.
+  #
+  # @return [void]
+  def self.configure!
+    unless @configured
+      RSpec.configure do |config|
+        config.before(:each) do |example|
+          leaks_cleaned = Metasploit::Framework::Spec::Constants.clean
+
+          if leaks_cleaned
+            $stderr.puts "Cleaned leaked constants before #{example.metadata.full_description}"
+          end
+
+          # clean so that leaks from earlier example aren't attributed to this example
+          Metasploit::Framework::Spec::Constants::Each.leaks_cleaned ||= leaks_cleaned
+        end
+
+        config.after(:each) do |example|
+          child_names = Metasploit::Framework::Spec::Constants.to_enum(:each).to_a
+
+          if child_names.length > 0
+            lines = ['Leaked constants:']
+
+            child_names.sort.each do |child_name|
+              lines << "  #{child_name}"
+            end
+
+            lines << ''
+            lines << "Add `include_context 'Metasploit::Framework::Spec::Constants cleaner'` to clean up constants from #{example.metadata.full_description}"
+
+            message = lines.join("\n")
+
+            # use caller metadata so that Jump to Source in the Rubymine RSpec running jumps to the example instead of
+            # here
+            fail RuntimeError, message, example.metadata[:caller]
+          end
+        end
+
+        config.after(:suite) do
+          if Metasploit::Framework::Spec::Constants::Each.leaks_cleaned?
+            if LOG_PATHNAME.exist?
+              LOG_PATHNAME.delete
+            end
+          else
+            LOG_PATHNAME.open('w') { |f|
+              f.puts "No leaks were cleaned by `Metasploit::Framework::Spec::Constants::Each.configured!`.  Remove " \
+                     "it from `spec/spec_helper.rb` so it does not interfere with contexts that persist loaded " \
+                     "modules for entire context and clean up modules in `after(:all)`"
+            }
+          end
+        end
+      end
+
+      @configured = true
+    end
+  end
+
+  # Whether {configure!} was called
+  #
+  # @return [Boolean]
+  def self.configured?
+    !!@configured
+  end
+
+  # Adds action to `spec` task so that `rake spec` fails if {configured!} is unnecessary in `spec/spec_helper.rb` and
+  # should be removed
+  #
+  # @return [void]
+  def self.define_task
+    Rake::Task.define_task('metasploit:framework:spec:constant:each:clean') do
+      if LOG_PATHNAME.exist?
+        LOG_PATHNAME.delete
+      end
+    end
+
+    Rake::Task.define_task(spec: 'metasploit:framework:spec:constant:each:clean')
+
+    Rake::Task.define_task(:spec) do
+      if LOG_PATHNAME.exist?
+        LOG_PATHNAME.open { |f|
+          f.each_line do |line|
+            $stderr.write line
+          end
+        }
+
+        exit(1)
+      end
+    end
+  end
+end

lib/metasploit/framework/spec/constants/suite.rb

@@ -0,0 +1,119 @@
+# Logs if constants created by module loading are left over after suite has completed.
+module Metasploit::Framework::Spec::Constants::Suite
+  #
+  # CONSTANTS
+  #
+
+  LOGS_PATHNAME = Pathname.new('log/metasploit/framework/spec/constants/suite')
+
+  # Logs leaked constants to {LOG_PATHNAME} and prints `message` to stderr.
+  #
+  # @param hook (see log_pathname)
+  # @param message [String] additional message printed to stderr when there is at least one leaked constant.
+  # @return [void]
+  def self.log_leaked_constants(hook, message)
+    count = 0
+    hook_log_pathname = log_pathname(hook)
+    hook_log_pathname.parent.mkpath
+
+    hook_log_pathname.open('w') do |f|
+      count = Metasploit::Framework::Spec::Constants.each do |child_name|
+        f.puts child_name
+      end
+    end
+
+    if count > 0
+      $stderr.puts "#{count} #{'constant'.pluralize(count)} leaked under " \
+                   "#{Metasploit::Framework::Spec::Constants::PARENT_CONSTANT}. #{message} See #{hook_log_pathname} " \
+                   "for details."
+    else
+      hook_log_pathname.delete
+    end
+  end
+
+  # Configures after(:suite) callback for RSpec to check for leaked constants.
+  def self.configure!
+    unless @configured
+      RSpec.configure do |config|
+        config.before(:suite) do
+          Metasploit::Framework::Spec::Constants::Suite.log_leaked_constants(
+              :before,
+              'Modules are being loaded outside callbacks before suite starts.'
+          )
+        end
+
+        config.after(:suite) do
+          Metasploit::Framework::Spec::Constants::Suite.log_leaked_constants(
+              :after,
+              'Modules are being loaded inside callbacks or examples during suite run.'
+          )
+        end
+      end
+
+      @configured = true
+    end
+  end
+
+  # Adds action to `spec` task so that `rake spec` fails if `log/leaked-constants.log` exists after printing out the
+  # leaked constants.
+  #
+  # @return [void]
+  def self.define_task
+    Rake::Task.define_task(:spec) do
+      leaked_before = Metasploit::Framework::Spec::Constants::Suite.print_leaked_constants(:before)
+      leaked_after = Metasploit::Framework::Spec::Constants::Suite.print_leaked_constants(:after)
+
+      # leaks after suite can be be cleaned up by {Metasploit::Framework::Spec::Constants::Each.configure!}, but
+      # leaks before suite require user intervention to find the leaks since it's a programming error in how the specs
+      # are written where Modules are being loaded in the context scope.
+      if leaked_after
+        $stderr.puts
+        $stderr.puts "Add `Metasploit::Framework::Spec::Constants::Each.configure!` to `spec/spec_helper.rb` " \
+                     "**NOTE: `Metasploit::Framework::Spec::Constants::Each` may report false leaks if `after(:all)` " \
+                     "is used to clean up constants instead of `after(:each)`**"
+      end
+
+      if leaked_before || leaked_after
+        exit 1
+      end
+    end
+  end
+
+  # @param hook [:after, :before] Whether the log is recording leaked constants `:before` the suite runs or `:after` the
+  #   suite runs.
+  def self.log_pathname(hook)
+    LOGS_PATHNAME.join("#{hook}.log")
+  end
+
+  # Prints logged leaked constants to stderr.
+  #
+  # @param hook [:after, :before] Whether the log is recording leaked constants `:before` the suite runs or `:after` the
+  #   suite runs.
+  # @return [true] if leaks printed
+  # @return [false] otherwise
+  def self.print_leaked_constants(hook)
+    hook_log_pathname = log_pathname(hook)
+
+    leaks = false
+
+    if hook_log_pathname.exist?
+      leaks = true
+      $stderr.puts "Leaked constants detected under #{Metasploit::Framework::Spec::Constants::PARENT_CONSTANT} #{hook} suite:"
+
+      hook_log_pathname.open do |f|
+        f.each_line do |line|
+          constant_name = line.strip
+          full_name = Metasploit::Framework::Spec::Constants.full_name(constant_name)
+
+          if full_name
+            formatted_full_name = " # #{full_name}"
+          end
+
+          $stderr.puts "  #{constant_name}#{formatted_full_name}"
+        end
+      end
+    end
+
+    leaks
+  end
+end