fix some race conditions (#5220)

* fix memory leak

* race condition fixes and change log

* race condition fixes and change log

hapi-fhir-base/src/main/java/ca/uhn/fhir/context/FhirContext.java

@@ -1177,7 +1177,14 @@ public class FhirContext {
 			synchronized (this) {
 				if (!myInitialized && !myInitializing) {
 					myInitializing = true;
-					scanResourceTypes(toElementList(myResourceTypesToScan));
+					try {
+						scanResourceTypes(toElementList(myResourceTypesToScan));
+					} catch (Exception e) {
+						ourLog.error("Failed to initialize FhirContext", e);
+						throw e;
+					} finally {
+						myInitializing = false;
+					}
 				}
 			}
 		}

hapi-fhir-cli/hapi-fhir-cli-api/src/main/java/ca/uhn/fhir/cli/BaseApp.java

@@ -270,14 +270,6 @@ public abstract class BaseApp {
 
 			// Actually execute the command
 			command.run(parsedOptions);
-
-			myShutdownHookHasNotRun = true;
-			runCleanupHookAndUnregister();
-
-			if (!HapiSystemProperties.isTestModeEnabled()) {
-				System.exit(0);
-			}
-
 		} catch (ParseException e) {
 			if (!HapiSystemProperties.isTestModeEnabled()) {
 				LogbackUtil.loggingConfigOff();
@@ -296,6 +288,13 @@ public abstract class BaseApp {
 			ourLog.error("Error during execution: ", t);
 			runCleanupHookAndUnregister();
 			exitDueToException(new CommandFailureException("Error: " + t, t));
+		} finally {
+			myShutdownHookHasNotRun = true;
+			runCleanupHookAndUnregister();
+
+			if (!HapiSystemProperties.isTestModeEnabled()) {
+				System.exit(0);
+			}
 		}
 	}
 

hapi-fhir-client/src/main/java/ca/uhn/fhir/rest/client/impl/RestfulClientFactory.java

@@ -75,8 +75,7 @@ public abstract class RestfulClientFactory implements IRestfulClientFactory {
 	/**
 	 * Constructor
 	 *
-	 * @param theFhirContext
+	 * @param theFhirContext The context
-	 *           The context
 	 */
 	public RestfulClientFactory(FhirContext theFhirContext) {
 		myContext = theFhirContext;
@@ -142,13 +141,10 @@ public abstract class RestfulClientFactory implements IRestfulClientFactory {
 	/**
 	 * Instantiates a new client instance
 	 *
-	 * @param theClientType
+	 * @param theClientType The client type, which is an interface type to be instantiated
-	 *           The client type, which is an interface type to be instantiated
+	 * @param theServerBase The URL of the base for the restful FHIR server to connect to
-	 * @param theServerBase
-	 *           The URL of the base for the restful FHIR server to connect to
 	 * @return A newly created client
-	 * @throws ConfigurationException
+	 * @throws ConfigurationException If the interface type is not an interface
-	 *            If the interface type is not an interface
 	 */
 	@Override
 	public synchronized <T extends IRestfulClient> T newClient(Class<T> theClientType, String theServerBase) {
@@ -281,13 +277,8 @@ public abstract class RestfulClientFactory implements IRestfulClientFactory {
 				break;
 
 			case ONCE:
-				if (myValidatedServerBaseUrls.contains(serverBase)) {
-					break;
-				}
-
 				synchronized (myValidatedServerBaseUrls) {
-					if (!myValidatedServerBaseUrls.contains(serverBase)) {
+					if (myValidatedServerBaseUrls.add(serverBase)) {
-						myValidatedServerBaseUrls.add(serverBase);
 						validateServerBase(serverBase, theHttpClient, theClient);
 					}
 				}
@@ -396,20 +387,13 @@ public abstract class RestfulClientFactory implements IRestfulClientFactory {
 		}
 
 		String serverBase = normalizeBaseUrlForMap(theServerBase);
-		if (myValidatedServerBaseUrls.contains(serverBase)) {
+		myValidatedServerBaseUrls.add(serverBase);
-			return;
-		}
-
-		synchronized (myValidatedServerBaseUrls) {
-			myValidatedServerBaseUrls.add(serverBase);
-		}
 	}
 
 	/**
 	 * Get the http client for the given server base
 	 *
-	 * @param theServerBase
+	 * @param theServerBase the server base
-	 *           the server base
 	 * @return the http client
 	 */
 	protected abstract IHttpClient getHttpClient(String theServerBase);

hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_0_0/5220-race-conditions.yaml

@@ -0,0 +1,7 @@
+---
+type: fix
+issue: 5220
+title: "Fixed a race condition in RestfulClientFactory that could cause validateInitialized() to deadlock.
+Fixed a race condition in FhirContext initialization that could produce a 'this.myNameToResourceType is null' NPE.
+Fixed a shutdown hook memory leak in BaseApp that happened when the command threw an exception;
+this memory leak only affects code that calls App.main repeatedly which is probably only in test code."

hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/client/ClientThreadedCapabilitiesTest.java

@@ -113,7 +113,7 @@ public class ClientThreadedCapabilitiesTest {
 			} else {
 				// metadata request must always be first
 				if (counter.get() == 0) {
-					fail("A non-metadata request was executed before metadata request");
+					fail("A non-metadata request was executed before metadata request: " + theRequest.getUri() + " counter: " + counter.get());
 				}
 			}
 		}