spring-cloud-openfeign icon indicating copy to clipboard operation
spring-cloud-openfeign copied to clipboard

@Cacheable doesnt work with sync=true

Open misostc opened this issue 3 years ago • 3 comments

Describe the bug When using annotation @Cacheable(value="cache-name", sync=true) and using client from multiple threads, the cache fails on exception


java.util.concurrent.ExecutionException: java.lang.IllegalStateException: Recursive update

	at java.base/java.util.concurrent.FutureTask.report(FutureTask.java:122)
	at java.base/java.util.concurrent.FutureTask.get(FutureTask.java:191)
	at org.springframework.cloud.openfeign.FeignClientCacheTests$givenSyncCached.cacheWorksFromTwoThreads(FeignClientCacheTests.java:116)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:568)
	at org.junit.platform.commons.util.ReflectionUtils.invokeMethod(ReflectionUtils.java:725)
	at org.junit.jupiter.engine.execution.MethodInvocation.proceed(MethodInvocation.java:60)
	at org.junit.jupiter.engine.execution.InvocationInterceptorChain$ValidatingInvocation.proceed(InvocationInterceptorChain.java:131)
	at org.junit.jupiter.engine.extension.TimeoutExtension.intercept(TimeoutExtension.java:149)
	at org.junit.jupiter.engine.extension.TimeoutExtension.interceptTestableMethod(TimeoutExtension.java:140)
	at org.junit.jupiter.engine.extension.TimeoutExtension.interceptTestMethod(TimeoutExtension.java:84)
	at org.junit.jupiter.engine.execution.ExecutableInvoker$ReflectiveInterceptorCall.lambda$ofVoidMethod$0(ExecutableInvoker.java:115)
	at org.junit.jupiter.engine.execution.ExecutableInvoker.lambda$invoke$0(ExecutableInvoker.java:105)
	at org.junit.jupiter.engine.execution.InvocationInterceptorChain$InterceptedInvocation.proceed(InvocationInterceptorChain.java:106)
	at org.junit.jupiter.engine.execution.InvocationInterceptorChain.proceed(InvocationInterceptorChain.java:64)
	at org.junit.jupiter.engine.execution.InvocationInterceptorChain.chainAndInvoke(InvocationInterceptorChain.java:45)
	at org.junit.jupiter.engine.execution.InvocationInterceptorChain.invoke(InvocationInterceptorChain.java:37)
	at org.junit.jupiter.engine.execution.ExecutableInvoker.invoke(ExecutableInvoker.java:104)
	at org.junit.jupiter.engine.execution.ExecutableInvoker.invoke(ExecutableInvoker.java:98)
	at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.lambda$invokeTestMethod$7(TestMethodTestDescriptor.java:214)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.invokeTestMethod(TestMethodTestDescriptor.java:210)
	at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.execute(TestMethodTestDescriptor.java:135)
	at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.execute(TestMethodTestDescriptor.java:66)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$6(NodeTestTask.java:151)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:141)
	at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:137)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$9(NodeTestTask.java:139)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:138)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:95)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
	at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.invokeAll(SameThreadHierarchicalTestExecutorService.java:41)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$6(NodeTestTask.java:155)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:141)
	at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:137)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$9(NodeTestTask.java:139)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:138)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:95)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
	at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.invokeAll(SameThreadHierarchicalTestExecutorService.java:41)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$6(NodeTestTask.java:155)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:141)
	at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:137)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$9(NodeTestTask.java:139)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:138)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:95)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
	at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.invokeAll(SameThreadHierarchicalTestExecutorService.java:41)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$6(NodeTestTask.java:155)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:141)
	at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:137)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$9(NodeTestTask.java:139)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:138)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:95)
	at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.submit(SameThreadHierarchicalTestExecutorService.java:35)
	at org.junit.platform.engine.support.hierarchical.HierarchicalTestExecutor.execute(HierarchicalTestExecutor.java:57)
	at org.junit.platform.engine.support.hierarchical.HierarchicalTestEngine.execute(HierarchicalTestEngine.java:54)
	at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:107)
	at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:88)
	at org.junit.platform.launcher.core.EngineExecutionOrchestrator.lambda$execute$0(EngineExecutionOrchestrator.java:54)
	at org.junit.platform.launcher.core.EngineExecutionOrchestrator.withInterceptedStreams(EngineExecutionOrchestrator.java:67)
	at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:52)
	at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:114)
	at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:86)
	at org.junit.platform.launcher.core.DefaultLauncherSession$DelegatingLauncher.execute(DefaultLauncherSession.java:86)
	at org.junit.platform.launcher.core.SessionPerRequestLauncher.execute(SessionPerRequestLauncher.java:53)
	at com.intellij.junit5.JUnit5IdeaTestRunner.startRunnerWithArgs(JUnit5IdeaTestRunner.java:71)
	at com.intellij.rt.junit.IdeaTestRunner$Repeater$1.execute(IdeaTestRunner.java:38)
	at com.intellij.rt.execution.junit.TestsRepeater.repeat(TestsRepeater.java:11)
	at com.intellij.rt.junit.IdeaTestRunner$Repeater.startRunnerWithArgs(IdeaTestRunner.java:35)
	at com.intellij.rt.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:235)
	at com.intellij.rt.junit.JUnitStarter.main(JUnitStarter.java:54)
Caused by: java.lang.IllegalStateException: Recursive update
	at java.base/java.util.concurrent.ConcurrentHashMap.computeIfAbsent(ConcurrentHashMap.java:1763)
	at org.springframework.cache.concurrent.ConcurrentMapCache.get(ConcurrentMapCache.java:142)
	at org.springframework.cache.interceptor.CacheAspectSupport.handleSynchronizedGet(CacheAspectSupport.java:442)
	at org.springframework.cache.interceptor.CacheAspectSupport.execute(CacheAspectSupport.java:382)
	at org.springframework.cache.interceptor.CacheAspectSupport.execute(CacheAspectSupport.java:345)
	at org.springframework.cache.interceptor.CacheInterceptor.invoke(CacheInterceptor.java:64)
	at org.springframework.cloud.openfeign.FeignCachingInvocationHandlerFactory.lambda$create$1(FeignCachingInvocationHandlerFactory.java:53)
	at org.springframework.cloud.openfeign.$Proxy125.getWithSyncedCache(Unknown Source)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:568)
	at org.springframework.aop.support.AopUtils.invokeJoinpointUsingReflection(AopUtils.java:343)
	at org.springframework.aop.framework.ReflectiveMethodInvocation.invokeJoinpoint(ReflectiveMethodInvocation.java:196)
	at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:163)
	at org.springframework.cache.interceptor.CacheInterceptor.lambda$invoke$0(CacheInterceptor.java:54)
	at org.springframework.cache.interceptor.CacheAspectSupport.invokeOperation(CacheAspectSupport.java:366)
	at org.springframework.cache.interceptor.CacheAspectSupport.lambda$handleSynchronizedGet$1(CacheAspectSupport.java:447)
	at org.springframework.cache.concurrent.ConcurrentMapCache.lambda$get$0(ConcurrentMapCache.java:144)
	at java.base/java.util.concurrent.ConcurrentHashMap.computeIfAbsent(ConcurrentHashMap.java:1708)
	at org.springframework.cache.concurrent.ConcurrentMapCache.get(ConcurrentMapCache.java:142)
	at org.springframework.cache.interceptor.CacheAspectSupport.handleSynchronizedGet(CacheAspectSupport.java:442)
	at org.springframework.cache.interceptor.CacheAspectSupport.execute(CacheAspectSupport.java:382)
	at org.springframework.cache.interceptor.CacheAspectSupport.execute(CacheAspectSupport.java:345)
	at org.springframework.cache.interceptor.CacheInterceptor.invoke(CacheInterceptor.java:64)
	at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:184)
	at org.springframework.aop.framework.JdkDynamicAopProxy.invoke(JdkDynamicAopProxy.java:215)
	at org.springframework.cloud.openfeign.$Proxy127.getWithSyncedCache(Unknown Source)
	at org.springframework.cloud.openfeign.FeignClientCacheTests$givenSyncCached.lambda$cacheWorksFromTwoThreads$0(FeignClientCacheTests.java:113)
	at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635)
	at java.base/java.lang.Thread.run(Thread.java:833)

Sample

I adjusted FeignClientCacheTests.java to show the problem (excuse the incomplete test, its just to show the exception is thrown with feign and not with regular spring component)

// src/test/java/org/springframework/cloud/openfeign/FeignClientCacheTests.java

/*
 * Copyright 2020-2020 the original author or authors.
 *
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
 * You may obtain a copy of the License at
 *
 *      https://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */

package org.springframework.cloud.openfeign;

import java.net.UnknownHostException;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.Future;

import feign.Contract;
import feign.RequestLine;
import feign.RetryableException;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.autoconfigure.EnableAutoConfiguration;
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.cache.CacheManager;
import org.springframework.cache.annotation.Cacheable;
import org.springframework.cache.annotation.EnableCaching;
import org.springframework.cache.interceptor.SimpleKey;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.test.annotation.DirtiesContext;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;

/**
 * @author Sam Kruglov
 */
@SpringBootTest(classes = FeignClientCacheTests.TestConfiguration.class)
@DirtiesContext
public class FeignClientCacheTests {

	private static final String CACHE_NAME = "foo-cache";
	private static final String CACHE_NAME_SYNC = "foo-cache-sync";

	@Autowired
	private FooClient foo;

	@Test
	void cacheExists(@Autowired CacheManager cacheManager) {
		assertThat(cacheManager.getCache(CACHE_NAME)).isNotNull();
	}

	@Test
	void interceptedCallsReal() {
		assertThatExceptionOfType(RetryableException.class).isThrownBy(foo::getWithCache)
				.withRootCauseInstanceOf(UnknownHostException.class);
	}

	@Test
	void nonInterceptedCallsReal() {
		assertThatExceptionOfType(RetryableException.class).isThrownBy(foo::getWithoutCache)
				.withRootCauseInstanceOf(UnknownHostException.class);
	}

	@Nested
	class givenCached {

		String cachedValue = "cached";

		@BeforeEach
		void setUp(@Autowired CacheManager cacheManager) {
			cacheManager.getCache(CACHE_NAME).put(SimpleKey.EMPTY, cachedValue);
		}

		@Test
		void interceptedReturnsCached() {
			assertThat(foo.getWithCache()).isSameAs(cachedValue);
		}

		@Test
		void nonInterceptedCallsReal() {
			assertThatExceptionOfType(RetryableException.class).isThrownBy(foo::getWithoutCache)
					.withRootCauseInstanceOf(UnknownHostException.class);
		}

	}

	@Nested
	class givenSyncCached {

		@Autowired
		CacheTest cacheTest;

		@Test
		void cacheWorksFromTwoThreads() throws ExecutionException, InterruptedException {

			ExecutorService executorService = Executors.newFixedThreadPool(2);

			Future<String> f1 = executorService.submit(() -> foo.getWithSyncedCache());
			Future<String> f2 = executorService.submit(() -> foo.getWithSyncedCache());

			assertThat(f1.get()).isEqualTo(f2.get());

		}
		@Test
		void cacheWorksFromTwoThreadsNoFeign() throws ExecutionException, InterruptedException {

			ExecutorService executorService = Executors.newFixedThreadPool(2);

			Future<String> f1 = executorService.submit(() -> cacheTest.test());
			Future<String> f2 = executorService.submit(() -> cacheTest.test());

			assertThat(f1.get()).isEqualTo(f2.get());
		}
	}

	@Configuration(proxyBeanMethods = false)
	@EnableFeignClients(clients = FooClient.class)
	@EnableAutoConfiguration
	@EnableCaching
	protected static class TestConfiguration {

		@Bean
		CacheTest cacheTest() {
			return new CacheTest();
		}

	}

	static class CacheTest {
		@Cacheable(cacheNames = "cache-not-feign", sync = true)
		public String test() throws InterruptedException {
			Thread.sleep(500L);
			return "test";
		}
	}

	@FeignClient(name = "foo", url = "http://foo", configuration = FooConfiguration.class)
	interface FooClient {

		@RequestLine("GET /with-cache")
		@Cacheable(cacheNames = CACHE_NAME)
		String getWithCache();

		@RequestLine("GET /with-cache-sync")
		@Cacheable(cacheNames = CACHE_NAME_SYNC, sync = true)
		default String getWithSyncedCache() throws InterruptedException {
			Thread.sleep(500L);
			return "test";
		}

		@RequestLine("GET /without-cache")
		String getWithoutCache();

	}

	public static class FooConfiguration {

		@Bean
		Contract feignContract() {
			return new Contract.Default();
		}

	}

}

misostc avatar Jan 06 '22 11:01 misostc

Hello, @misostc, thanks for creating the issue. Looks like a bug.

OlgaMaciaszek avatar Jan 17 '22 15:01 OlgaMaciaszek

Sorry to bump this, but are there any news on this issue? I also ran into this problem today :(

BenniG82 avatar Jun 22 '22 19:06 BenniG82

I have a similar problem, threads lock with sync = true. Without this option, all works fine.

My workaround was to deactivate feign cache feature with the configuration feign.cache.enabled=false

emmanuel-vasseur avatar Sep 27 '22 20:09 emmanuel-vasseur