De-couple ServiceLoader calls from class loading

Use the holder pattern to de-couple the loading of super classes from
the ServiceLoader calls to set up global instances. This prevents
potential lock inversions.

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

2 files changed, 65 insertions, 28 deletions

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/GpgSignatureVerifierFactory.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/GpgSignatureVerifierFactory.java
index 4b1dbedeb1..59775c475b 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/GpgSignatureVerifierFactory.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/GpgSignatureVerifierFactory.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2021, Thomas Wolf <thomas.wolf@paranor.ch> and others
+ * Copyright (C) 2021, 2022 Thomas Wolf <thomas.wolf@paranor.ch> 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,20 +26,41 @@ public abstract class GpgSignatureVerifierFactory {
 	private static final Logger LOG = LoggerFactory
 			.getLogger(GpgSignatureVerifierFactory.class);
 
-	private static volatile GpgSignatureVerifierFactory defaultFactory = loadDefault();
+	private static class DefaultFactory {
 
-	private static GpgSignatureVerifierFactory loadDefault() {
-		try {
-			ServiceLoader<GpgSignatureVerifierFactory> loader = ServiceLoader
-					.load(GpgSignatureVerifierFactory.class);
-			Iterator<GpgSignatureVerifierFactory> iter = loader.iterator();
-			if (iter.hasNext()) {
-				return iter.next();
+		private static volatile GpgSignatureVerifierFactory defaultFactory = loadDefault();
+
+		private static GpgSignatureVerifierFactory loadDefault() {
+			try {
+				ServiceLoader<GpgSignatureVerifierFactory> loader = ServiceLoader
+						.load(GpgSignatureVerifierFactory.class);
+				Iterator<GpgSignatureVerifierFactory> 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 DefaultFactory() {
+			// No instantiation
+		}
+
+		public static GpgSignatureVerifierFactory getDefault() {
+			return defaultFactory;
+		}
+
+		/**
+		 * Sets the default factory.
+		 *
+		 * @param factory
+		 *            the new default factory
+		 */
+		public static void setDefault(GpgSignatureVerifierFactory factory) {
+			defaultFactory = factory;
 		}
-		return null;
 	}
 
 	/**
@@ -48,7 +69,7 @@ public abstract class GpgSignatureVerifierFactory {
 	 * @return the default factory or {@code null} if none set
 	 */
 	public static GpgSignatureVerifierFactory getDefault() {
-		return defaultFactory;
+		return DefaultFactory.getDefault();
 	}
 
 	/**
@@ -58,7 +79,7 @@ public abstract class GpgSignatureVerifierFactory {
 	 *            the new default factory
 	 */
 	public static void setDefault(GpgSignatureVerifierFactory factory) {
-		defaultFactory = factory;
+		DefaultFactory.setDefault(factory);
 	}
 
 	/**
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/SshSessionFactory.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/SshSessionFactory.java
index 1e98a56f79..a0194ea8b1 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/SshSessionFactory.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/SshSessionFactory.java
@@ -1,6 +1,6 @@
 /*
  * Copyright (C) 2008, Robin Rosenberg <robin.rosenberg@dewire.com>
- * Copyright (C) 2008, 2020 Shawn O. Pearce <spearce@spearce.org> and others
+ * Copyright (C) 2008, 2022 Shawn O. Pearce <spearce@spearce.org> 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
@@ -36,15 +36,35 @@ import org.eclipse.jgit.util.SystemReader;
  */
 public abstract class SshSessionFactory {
 
-	private static volatile SshSessionFactory INSTANCE = loadSshSessionFactory();
+	private static class DefaultFactory {
 
-	private static SshSessionFactory loadSshSessionFactory() {
-		ServiceLoader<SshSessionFactory> loader = ServiceLoader.load(SshSessionFactory.class);
-		Iterator<SshSessionFactory> iter = loader.iterator();
-		if(iter.hasNext()) {
-			return iter.next();
+		private static volatile SshSessionFactory INSTANCE = loadSshSessionFactory();
+
+		private static SshSessionFactory loadSshSessionFactory() {
+			ServiceLoader<SshSessionFactory> loader = ServiceLoader
+					.load(SshSessionFactory.class);
+			Iterator<SshSessionFactory> iter = loader.iterator();
+			if (iter.hasNext()) {
+				return iter.next();
+			}
+			return null;
+		}
+
+		private DefaultFactory() {
+			// No instantiation
+		}
+
+		public static SshSessionFactory getInstance() {
+			return INSTANCE;
+		}
+
+		public static void setInstance(SshSessionFactory newFactory) {
+			if (newFactory != null) {
+				INSTANCE = newFactory;
+			} else {
+				INSTANCE = loadSshSessionFactory();
+			}
 		}
-		return null;
 	}
 
 	/**
@@ -57,7 +77,7 @@ public abstract class SshSessionFactory {
 	 * @return factory the current factory for this JVM.
 	 */
 	public static SshSessionFactory getInstance() {
-		return INSTANCE;
+		return DefaultFactory.getInstance();
 	}
 
 	/**
@@ -68,11 +88,7 @@ public abstract class SshSessionFactory {
 	 *            {@code null} the default factory will be restored.
 	 */
 	public static void setInstance(SshSessionFactory newFactory) {
-		if (newFactory != null) {
-			INSTANCE = newFactory;
-		} else {
-			INSTANCE = loadSshSessionFactory();
-		}
+		DefaultFactory.setInstance(newFactory);
 	}
 
 	/**