From 8eb350663e0dbee481e5a5f2a785dbbf365e18cc Mon Sep 17 00:00:00 2001
From: Jacob Vosmaer <jacob@gitlab.com>
Date: Thu, 30 Mar 2017 16:26:30 +0200
Subject: [PATCH] Deal with Rails autoload instance variable resets

Rails auto-load (a development feature) can end up resetting instance
variables on classes. This breaks Gitlab::GitalyClient, which uses
instance variables to keep global hashes to look up channels and
addresses. This change adds code that regenerates the hashes if they
suddenly become nil.
---
 config/initializers/8_gitaly.rb | 14 +-----------
 lib/gitlab/gitaly_client.rb     | 38 ++++++++++++++++++++++++++++-----
 2 files changed, 34 insertions(+), 18 deletions(-)

diff --git a/config/initializers/8_gitaly.rb b/config/initializers/8_gitaly.rb
index c7f27c78535..42ec7240b0f 100644
--- a/config/initializers/8_gitaly.rb
+++ b/config/initializers/8_gitaly.rb
@@ -2,17 +2,5 @@ require 'uri'
 
 # Make sure we initialize our Gitaly channels before Sidekiq starts multi-threaded execution.
 if Gitlab.config.gitaly.enabled || Rails.env.test?
-  Gitlab.config.repositories.storages.each do |name, params|
-    address = params['gitaly_address']
-
-    unless address.present?
-      raise "storage #{name.inspect} is missing a gitaly_address"
-    end
-
-    unless URI(address).scheme.in?(%w(tcp unix))
-      raise "Unsupported Gitaly address: #{address.inspect}"
-    end
-
-    Gitlab::GitalyClient.configure_channel(name, address)
-  end
+  Gitlab::GitalyClient.configure_channels
 end
diff --git a/lib/gitlab/gitaly_client.rb b/lib/gitlab/gitaly_client.rb
index fe15fb12adb..bcdf1b1faa8 100644
--- a/lib/gitlab/gitaly_client.rb
+++ b/lib/gitlab/gitaly_client.rb
@@ -4,11 +4,23 @@ module Gitlab
   module GitalyClient
     SERVER_VERSION_FILE = 'GITALY_SERVER_VERSION'.freeze
 
-    def self.configure_channel(storage, address)
-      @addresses ||= {}
-      @addresses[storage] = address
-      @channels ||= {}
-      @channels[storage] = new_channel(address)
+    # This function is not thread-safe because it updates Hashes in instance variables.
+    def self.configure_channels
+      @addresses = {}
+      @channels = {}
+      Gitlab.config.repositories.storages.each do |name, params|
+        address = params['gitaly_address']
+        unless address.present?
+          raise "storage #{name.inspect} is missing a gitaly_address"
+        end
+
+        unless URI(address).scheme.in?(%w(tcp unix))
+          raise "Unsupported Gitaly address: #{address.inspect}"
+        end
+
+        @addresses[name] = address
+        @channels[name] = new_channel(address)
+      end
     end
 
     def self.new_channel(address)
@@ -21,10 +33,26 @@ module Gitlab
     end
 
     def self.get_channel(storage)
+      if !Rails.env.production? && @channels.nil?
+        # In development mode the Rails auto-loader may reset the instance
+        # variables of this class. What we do here is not thread-safe. In normal
+        # circumstances (including production) these instance variables have
+        # been initialized from config/initializers.
+        configure_channels
+      end
+
       @channels[storage]
     end
 
     def self.get_address(storage)
+      if !Rails.env.production? && @addresses.nil?
+        # In development mode the Rails auto-loader may reset the instance
+        # variables of this class. What we do here is not thread-safe. In normal
+        # circumstances (including development) these instance variables have
+        # been initialized from config/initializers.
+        configure_channels
+      end
+
       @addresses[storage]
     end