GpgSigner: prevent class lock inversion on the default signer

Don't store the default signer in a static field of the abstract
superclass GpgSigner. This many lead to a lock inversion on the class
initialization locks if there are concurrent loads of the GpgSigner
class and of one of its subclasses, and that subclass happens to be
the one the ServiceLoader wants to load.

Use the holder pattern to de-couple the loading of class GpgSigner
from the ServiceLoader call.

Bug: 579550
Change-Id: Ifac0ea0c8985a09fe0518d0dabc072fafd6db907
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/GpgSigner.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/GpgSigner.java
index 5b32cf0b5fab80c1a949f4c833356b90f2d13f12..b25a61b506a6e987433967ec9a14fb04c2e61691 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/GpgSigner.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/GpgSigner.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2018, Salesforce. and others
+ * Copyright (C) 2018, 2022 Salesforce and others
  *
  * This program and the accompanying materials are made available under the
  * terms of the Eclipse Distribution License v. 1.0 which is available at
@@ -26,22 +26,38 @@ import org.slf4j.LoggerFactory;
  * @since 5.3
  */
 public abstract class GpgSigner {
+
        private static final Logger LOG = LoggerFactory.getLogger(GpgSigner.class);
 
-       private static GpgSigner defaultSigner = loadGpgSigner();
+       private static class DefaultSigner {
+
+               private static volatile GpgSigner defaultSigner = loadGpgSigner();
 
-       private static GpgSigner loadGpgSigner() {
-               try {
-                       ServiceLoader<GpgSigner> loader = ServiceLoader
-                                       .load(GpgSigner.class);
-                       Iterator<GpgSigner> iter = loader.iterator();
-                       if (iter.hasNext()) {
-                               return iter.next();
+               private static GpgSigner loadGpgSigner() {
+                       try {
+                               ServiceLoader<GpgSigner> loader = ServiceLoader
+                                               .load(GpgSigner.class);
+                               Iterator<GpgSigner> iter = loader.iterator();
+                               if (iter.hasNext()) {
+                                       return iter.next();
+                               }
+                       } catch (ServiceConfigurationError e) {
+                               LOG.error(e.getMessage(), e);
                        }
-               } catch (ServiceConfigurationError e) {
-                       LOG.error(e.getMessage(), e);
+                       return null;
+               }
+
+               private DefaultSigner() {
+                       // No instantiation
+               }
+
+               public static GpgSigner getDefault() {
+                       return defaultSigner;
+               }
+
+               public static void setDefault(GpgSigner signer) {
+                       defaultSigner = signer;
                }
-               return null;
        }
 
        /**
@@ -50,7 +66,7 @@ public abstract class GpgSigner {
         * @return the default signer, or <code>null</code>.
         */
        public static GpgSigner getDefault() {
-               return defaultSigner;
+               return DefaultSigner.getDefault();
        }
 
        /**
@@ -61,7 +77,7 @@ public abstract class GpgSigner {
         *            default.
         */
        public static void setDefault(GpgSigner signer) {
-               GpgSigner.defaultSigner = signer;
+               DefaultSigner.setDefault(signer);
        }
 
        /**