Optimize branch lookups and force a repository reload for Repository#find_branchsh-reload-find-branch

If `git gc` runs and `Repository` has an instance to `Rugged::Repository`, a
bug in libgit2 may cause the instance to return a stale value or a missing
branch. This change not only optimizes the branch lookup so we don't have
to iterate through every branch, but it also works around the `git gc`
issue by forcing a repository reload every time `Repository#find_branch`
is called.

See: https://github.com/libgit2/libgit2/issues/1534

Closes #15392, #21470

5 files changed, 39 insertions, 5 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 9837b2edb9d..9ab32684de6 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -60,6 +60,9 @@ v 8.12.0 (unreleased)
   - User can edit closed MR with deleted fork (Katarzyna Kobierska Ula Budziszewska) !5496
   - Fixed invisible scroll controls on build page on iPhone
 
+v 8.11.5 (unreleased)
+  - Optimize branch lookups and force a repository reload for Repository#find_branch
+
 v 8.11.4 (unreleased)
   - Fix broken gitlab:backup:restore because of bad permissions on repo storage !6098 (Dirk Hörner)
   - Fix sorting issues by "last updated" doesn't work after import from GitHub
diff --git a/Gemfile b/Gemfile
index 96841013815..620338e5997 100644
--- a/Gemfile
+++ b/Gemfile
@@ -53,7 +53,7 @@ gem 'browser', '~> 2.2'
 
 # Extracting information from a git repository
 # Provide access to Gitlab::Git library
-gem 'gitlab_git', '~> 10.4.7'
+gem 'gitlab_git', '~> 10.5'
 
 # LDAP Auth
 # GitLab fork with several improvements to original library. For full list of changes
diff --git a/Gemfile.lock b/Gemfile.lock
index 1d0fcfd3c3a..28ede86b3ba 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -279,7 +279,7 @@ GEM
       diff-lcs (~> 1.1)
       mime-types (>= 1.16, < 3)
       posix-spawn (~> 0.3)
-    gitlab_git (10.4.7)
+    gitlab_git (10.5.0)
       activesupport (~> 4.0)
       charlock_holmes (~> 0.7.3)
       github-linguist (~> 4.7.0)
@@ -858,7 +858,7 @@ DEPENDENCIES
   github-linguist (~> 4.7.0)
   github-markup (~> 1.4)
   gitlab-flowdock-git-hook (~> 1.0.1)
-  gitlab_git (~> 10.4.7)
+  gitlab_git (~> 10.5)
   gitlab_meta (= 7.0)
   gitlab_omniauth-ldap (~> 1.2.1)
   gollum-lib (~> 4.2)
diff --git a/app/models/repository.rb b/app/models/repository.rb
index 91bdafdac99..f891e8374d2 100644
--- a/app/models/repository.rb
+++ b/app/models/repository.rb
@@ -120,8 +120,21 @@ class Repository
     commits
   end
 
-  def find_branch(name)
-    raw_repository.branches.find { |branch| branch.name == name }
+  def find_branch(name, fresh_repo: true)
+    # Since the Repository object may have in-memory index changes, invalidating the memoized Repository object may
+    # cause unintended side effects. Because finding a branch is a read-only operation, we can safely instantiate
+    # a new repo here to ensure a consistent state to avoid a libgit2 bug where concurrent access (e.g. via git gc)
+    # may cause the branch to "disappear" erroneously or have the wrong SHA.
+    #
+    # See: https://github.com/libgit2/libgit2/issues/1534 and https://gitlab.com/gitlab-org/gitlab-ce/issues/15392
+    raw_repo =
+      if fresh_repo
+        Gitlab::Git::Repository.new(path_to_repo)
+      else
+        raw_repository
+      end
+
+    raw_repo.find_branch(name)
   end
 
   def find_tag(name)
diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb
index 1fea50ad42c..812c72c48cb 100644
--- a/spec/models/repository_spec.rb
+++ b/spec/models/repository_spec.rb
@@ -382,6 +382,24 @@ describe Repository, models: true do
     end
   end
 
+  describe '#find_branch' do
+    it 'loads a branch with a fresh repo' do
+      expect(Gitlab::Git::Repository).to receive(:new).twice.and_call_original
+
+      2.times do
+        expect(repository.find_branch('feature')).not_to be_nil
+      end
+    end
+
+    it 'loads a branch with a cached repo' do
+      expect(Gitlab::Git::Repository).to receive(:new).once.and_call_original
+
+      2.times do
+        expect(repository.find_branch('feature', fresh_repo: false)).not_to be_nil
+      end
+    end
+  end
+
   describe '#rm_branch' do
     let(:old_rev) { '0b4bc9a49b562e85de7cc9e834518ea6828729b9' } # git rev-parse feature
     let(:blank_sha) { '0000000000000000000000000000000000000000' }