Merge branch 'fix/thread-safe-gpgme-tmp-directory' into 'master'

changelogs/unreleased/fix-thread-safe-gpgme-tmp-directory.yml

+---
+title: Make GPGME temporary directory handling thread safe
+merge_request: 13481
+author: Alexis Reigel

lib/gitlab/gpg.rb

@@ -2,6 +2,8 @@ module Gitlab
   module Gpg
     extend self
 
+    MUTEX = Mutex.new
+
     module CurrentKeyChain
       extend self
 
@@ -42,21 +44,37 @@ module Gitlab
       end
     end
 
-    def using_tmp_keychain
-      Dir.mktmpdir do |dir|
-        @original_dirs ||= [GPGME::Engine.dirinfo('homedir')]
-        @original_dirs.push(dir)
-
-        GPGME::Engine.home_dir = dir
-
-        return_value = yield
+    # 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
 
-        @original_dirs.pop
+    # 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
 
-        GPGME::Engine.home_dir = @original_dirs[-1]
+    private
 
-        return_value
+    def optimistic_using_tmp_keychain
+      previous_dir = current_home_dir
+      Dir.mktmpdir do |dir|
+        GPGME::Engine.home_dir = dir
+        yield
       end
+    ensure
+      GPGME::Engine.home_dir = previous_dir
     end
   end
 end

spec/lib/gitlab/gpg_spec.rb

@@ -43,6 +43,58 @@ describe Gitlab::Gpg do
       ).to eq []
     end
   end
+
+  describe '.current_home_dir' do
+    let(:default_home_dir) { GPGME::Engine.dirinfo('homedir') }
+
+    it 'returns the default value when no explicit home dir has been set' do
+      expect(described_class.current_home_dir).to eq default_home_dir
+    end
+
+    it 'returns the explicitely set home dir' do
+      GPGME::Engine.home_dir = '/tmp/gpg'
+
+      expect(described_class.current_home_dir).to eq '/tmp/gpg'
+
+      GPGME::Engine.home_dir = GPGME::Engine.dirinfo('homedir')
+    end
+
+    it 'returns the default value when explicitely setting the home dir to nil' do
+      GPGME::Engine.home_dir = nil
+
+      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