use mutex for keychain interaction

setting of the gpg home directory is not thread safe, as the directoy
gets stored on the class.

if multiple threads change the directory at the same time, one of the
threads will be working in the wrong directory.

2 files changed, 56 insertions, 8 deletions

diff --git a/lib/gitlab/gpg.rb b/lib/gitlab/gpg.rb
index 653c56d925b..78ebd8866a1 100644
--- a/lib/gitlab/gpg.rb
+++ b/lib/gitlab/gpg.rb
@@ -2,6 +2,8 @@ module Gitlab
   module Gpg
     extend self
 
+    MUTEX = Mutex.new
+
     module CurrentKeyChain
       extend self
 
@@ -42,7 +44,30 @@ module Gitlab
       end
     end
 
-    def using_tmp_keychain
+    # Allows thread safe switching of temporary keychain files
+    #
+    # 1. The current thread may use nesting of temporary keychain
+    # 2. Another thread needs to wait for the lock to be released
+    def using_tmp_keychain(&block)
+      if MUTEX.locked? && MUTEX.owned?
+        optimistic_using_tmp_keychain(&block)
+      else
+        MUTEX.synchronize do
+          optimistic_using_tmp_keychain(&block)
+        end
+      end
+    end
+
+    # 1. Returns the custom home directory if one has been set by calling
+    #    `GPGME::Engine.home_dir=`
+    # 2. Returns the default home directory otherwise
+    def current_home_dir
+      GPGME::Engine.info.first.home_dir || GPGME::Engine.dirinfo('homedir')
+    end
+
+    private
+
+    def optimistic_using_tmp_keychain
       Dir.mktmpdir do |dir|
         previous_dir = current_home_dir
 
@@ -55,12 +80,5 @@ module Gitlab
         return_value
       end
     end
-
-    # 1. Returns the custom home directory if one has been set by calling
-    #    `GPGME::Engine.home_dir=`
-    # 2. Returns the default home directory otherwise
-    def current_home_dir
-      GPGME::Engine.info.first.home_dir || GPGME::Engine.dirinfo('homedir')
-    end
   end
 end
diff --git a/spec/lib/gitlab/gpg_spec.rb b/spec/lib/gitlab/gpg_spec.rb
index 95d371ea178..30ad033b204 100644
--- a/spec/lib/gitlab/gpg_spec.rb
+++ b/spec/lib/gitlab/gpg_spec.rb
@@ -65,6 +65,36 @@ describe Gitlab::Gpg do
       expect(described_class.current_home_dir).to eq default_home_dir
     end
   end
+
+  describe '.using_tmp_keychain' do
+    it "the second thread does not change the first thread's directory" do
+      thread1 = Thread.new do
+        described_class.using_tmp_keychain do
+          dir = described_class.current_home_dir
+          sleep 0.1
+          expect(described_class.current_home_dir).to eq dir
+        end
+      end
+
+      thread2 = Thread.new do
+        described_class.using_tmp_keychain do
+          sleep 0.2
+        end
+      end
+
+      thread1.join
+      thread2.join
+    end
+
+    it 'allows recursive execution in the same thread' do
+      expect do
+        described_class.using_tmp_keychain do
+          described_class.using_tmp_keychain do
+          end
+        end
+      end.not_to raise_error(ThreadError)
+    end
+  end
 end
 
 describe Gitlab::Gpg::CurrentKeyChain do