Avoid race condition in ServiceLoader (#4295)

* Avoid race condition in ServiceLoader

* Changelog
This commit is contained in:
James Agnew 2022-11-22 07:48:54 -05:00 committed by GitHub
parent 62c630c546
commit 9eeb1545a1
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 108 additions and 55 deletions

View File

@ -0,0 +1,6 @@
---
type: fix
issue: 4295
title: "A race condition in the Cache factory has been resolved. This issue could cause
strange errors such as NoSuchElementException if caches are created in multiple
threads."

View File

@ -8,40 +8,35 @@ import java.util.ServiceLoader;
@SuppressWarnings("unchecked")
public class CacheFactory {
@SuppressWarnings("rawtypes")
static ServiceLoader<CacheProvider> loader = ServiceLoader.load(CacheProvider.class);
public static Iterator<CacheProvider> providers(boolean refresh) {
if (refresh) {
loader.reload();
}
return loader.iterator();
}
public static <K, V> Cache<K, V> build(long timeoutMillis) {
if (providers(false).hasNext()) {
return providers(false).next().create(timeoutMillis);
@SuppressWarnings("rawtypes")
private static synchronized <K, V> CacheProvider<K, V> getCacheProvider() {
Iterator<CacheProvider> iterator = loader.iterator();
if (iterator.hasNext()) {
return iterator.next();
}
throw new RuntimeException(Msg.code(2200) + "No Cache Service Providers found. Choose between hapi-fhir-caching-caffeine (Default) and hapi-fhir-caching-guava (Android)");
}
public static <K, V> LoadingCache<K, V> build(long timeoutMillis, CacheLoader<K, V> cacheLoader) {
if (providers(false).hasNext()) {
return providers(false).next().create(timeoutMillis, cacheLoader);
}
throw new RuntimeException(Msg.code(2201) + "No Cache Service Providers found. Choose between hapi-fhir-caching-caffeine (Default) and hapi-fhir-caching-guava (Android)");
public static <K, V> Cache<K, V> build(long theTimeoutMillis) {
CacheProvider<Object, Object> cacheProvider = getCacheProvider();
return cacheProvider.create(theTimeoutMillis);
}
public static <K, V> Cache<K, V> build(long timeoutMillis, long maximumSize) {
if (providers(false).hasNext()) {
return providers(false).next().create(timeoutMillis, maximumSize);
}
throw new RuntimeException(Msg.code(2202) + "No Cache Service Providers found. Choose between hapi-fhir-caching-caffeine (Default) and hapi-fhir-caching-guava (Android)");
public static <K, V> LoadingCache<K, V> build(long theTimeoutMillis, CacheLoader<K, V> theCacheLoader) {
CacheProvider<K, V> cacheProvider = getCacheProvider();
return cacheProvider.create(theTimeoutMillis, theCacheLoader);
}
public static <K, V> LoadingCache<K, V> build(long timeoutMillis, long maximumSize, CacheLoader<K, V> cacheLoader) {
if (providers(false).hasNext()) {
return providers(false).next().create(timeoutMillis, maximumSize, cacheLoader);
}
throw new RuntimeException(Msg.code(2203) + "No Cache Service Providers found. Choose between hapi-fhir-caching-caffeine (Default) and hapi-fhir-caching-guava (Android)");
public static <K, V> Cache<K, V> build(long theTimeoutMillis, long theMaximumSize) {
CacheProvider<Object, Object> cacheProvider = getCacheProvider();
return cacheProvider.create(theTimeoutMillis, theMaximumSize);
}
public static <K, V> LoadingCache<K, V> build(long theTimeoutMillis, long theMaximumSize, CacheLoader<K, V> cacheLoader) {
CacheProvider<K, V> cacheProvider = getCacheProvider();
return cacheProvider.create(theTimeoutMillis, theMaximumSize, cacheLoader);
}
}

View File

@ -26,6 +26,12 @@
<groupId>com.github.ben-manes.caffeine</groupId>
<artifactId>caffeine</artifactId>
</dependency>
<dependency>
<groupId>ch.qos.logback</groupId>
<artifactId>logback-classic</artifactId>
<scope>test</scope>
</dependency>
</dependencies>
<build>

View File

@ -0,0 +1,59 @@
package ca.uhn.fhir.sl.cache.caffeine;
import ca.uhn.fhir.sl.cache.Cache;
import ca.uhn.fhir.sl.cache.CacheFactory;
import ca.uhn.fhir.sl.cache.LoadingCache;
import org.junit.jupiter.api.MethodOrderer;
import org.junit.jupiter.api.Order;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.TestMethodOrder;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.Future;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;
@TestMethodOrder(MethodOrderer.OrderAnnotation.class)
public class CacheLoaderTest {
private static final Logger ourLog = LoggerFactory.getLogger(CacheLoaderTest.class);
@Order(0)
@Test
public void testConcurrentInitialization() throws ExecutionException, InterruptedException, TimeoutException {
ExecutorService executor = Executors.newFixedThreadPool(5);
try {
List<Future<Cache<?, ?>>> futures = new ArrayList<>(5);
for (int i = 0; i < 5; i++) {
Future<Cache<?, ?>> future = executor.submit(() -> CacheFactory.build(1000));
futures.add(future);
}
for (var next : futures) {
Cache<?, ?> actual = next.get(1, TimeUnit.MINUTES);
ourLog.info("Got cache: {}", actual);
assertNotNull(actual);
}
} finally {
executor.shutdown();
}
}
@Order(1)
@Test
void loaderReturnsNullTest() {
LoadingCache<String, String> cache = CacheFactory.build(1000, key -> {
return null;
});
assertNull(cache.get("1"));
}
}

View File

@ -1,17 +0,0 @@
package ca.uhn.fhir.sl.cache.caffeine;
import static org.junit.jupiter.api.Assertions.assertNull;
import ca.uhn.fhir.sl.cache.CacheFactory;
import ca.uhn.fhir.sl.cache.LoadingCache;
import org.junit.jupiter.api.Test;
public class CacheLoaderTest {
@Test
void loaderReturnsNullTest() {
LoadingCache<String, String> cache = CacheFactory.build(1000, key -> {
return null;
});
assertNull(cache.get("1"));
}
}

View File

@ -1,13 +0,0 @@
package ca.uhn.fhir.sl.cache.caffeine;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import ca.uhn.fhir.sl.cache.CacheFactory;
import org.junit.jupiter.api.Test;
public class ServiceLoaderTest {
@Test
void loaderIsAvailable() {
assertNotNull(CacheFactory.build(1000));
}
}

View File

@ -0,0 +1,17 @@
<configuration>
<appender name="STDOUT" class="ch.qos.logback.core.ConsoleAppender">
<filter class="ch.qos.logback.classic.filter.ThresholdFilter">
<level>INFO</level>
</filter>
<encoder>
<pattern>%d{yyyy-MM-dd HH:mm:ss.SSS} [%thread] %-5level %logger{36} [%file:%line] %msg%n</pattern>
</encoder>
</appender>
<root level="info">
<appender-ref ref="STDOUT" />
</root>
</configuration>