From 782064167e696837a70f09ef5ab4ca76ff956293 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon <grzesiek.bizon@gmail.com> Date: Fri, 2 Jun 2023 14:24:43 +0200 Subject: [PATCH] Fix memory leak in CI config includes entry In GitLab 11.9, https://gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/24098 introduced validation of `include` keywords. However, it quietly introduced a memory leak: any time a new `include` entry was instantiated, a proc would be added to the `aspects` class variable. As the list grew, the time taken to process other `include` keywords would grow. This commit fixes this by doing away with the class variable in favor of `ComposableArray`. Changelog: fixed --- lib/gitlab/ci/config/entry/includes.rb | 14 +++----------- spec/lib/gitlab/ci/config/entry/includes_spec.rb | 16 ++++++++++++++++ 2 files changed, 19 insertions(+), 11 deletions(-) create mode 100644 spec/lib/gitlab/ci/config/entry/includes_spec.rb diff --git a/lib/gitlab/ci/config/entry/includes.rb b/lib/gitlab/ci/config/entry/includes.rb index 24d0e27e3a775ec7..671ecf4eac19c424 100644 --- a/lib/gitlab/ci/config/entry/includes.rb +++ b/lib/gitlab/ci/config/entry/includes.rb @@ -7,7 +7,7 @@ module Entry ## # Entry that represents a list of include. # - class Includes < ::Gitlab::Config::Entry::Node + class Includes < ::Gitlab::Config::Entry::ComposableArray include ::Gitlab::Config::Entry::Validatable validations do @@ -23,16 +23,8 @@ class Includes < ::Gitlab::Config::Entry::Node end end - def self.aspects - super.append -> do - @config = Array.wrap(@config) - - @config.each_with_index do |config, i| - @entries[i] = ::Gitlab::Config::Entry::Factory.new(Entry::Include) - .value(config || {}) - .create! - end - end + def composable_class + Entry::Include end end end diff --git a/spec/lib/gitlab/ci/config/entry/includes_spec.rb b/spec/lib/gitlab/ci/config/entry/includes_spec.rb new file mode 100644 index 0000000000000000..f1f28c24e70492b1 --- /dev/null +++ b/spec/lib/gitlab/ci/config/entry/includes_spec.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +require 'fast_spec_helper' +require_dependency 'active_model' + +RSpec.describe ::Gitlab::Ci::Config::Entry::Includes, feature_category: :pipeline_composition do + subject(:include_entry) { described_class.new(config) } + + describe '#initialize' do + let(:config) { 'test.yml' } + + it 'does not increase aspects' do + 2.times { expect { described_class.new(config) }.not_to change { described_class.aspects.count } } + end + end +end -- GitLab