From f4b3873d2fb905eb487dec80cb79cc2f216d8689 Mon Sep 17 00:00:00 2001 From: Phoebe0126 <1542756558@qq.com> Date: Thu, 7 May 2026 15:06:42 +0800 Subject: [PATCH] fix: add isAvailable check in getInvoker to prevent idle eviction race condition --- .../core/cluster/def/DefClusterInvoker.java | 5 +- .../cluster/def/DefClusterInvokerTest.java | 53 +++++++++++++++++++ 2 files changed, 57 insertions(+), 1 deletion(-) diff --git a/trpc-core/src/main/java/com/tencent/trpc/core/cluster/def/DefClusterInvoker.java b/trpc-core/src/main/java/com/tencent/trpc/core/cluster/def/DefClusterInvoker.java index 653559b98..bf3155f18 100644 --- a/trpc-core/src/main/java/com/tencent/trpc/core/cluster/def/DefClusterInvoker.java +++ b/trpc-core/src/main/java/com/tencent/trpc/core/cluster/def/DefClusterInvoker.java @@ -72,7 +72,10 @@ protected CompletionStage doInvoke(Request request, CompletionStage getInvoker(ServiceInstance instance) { String key = toUniqKey(instance); ConsumerInvokerProxy result = invokerCache.get(key); - return Optional.ofNullable(result).orElseGet(() -> createInvoker(instance)); + if (result != null && result.isAvailable()) { + return result; + } + return createInvoker(instance); } @SuppressWarnings("rawtypes") diff --git a/trpc-core/src/test/java/com/tencent/trpc/core/cluster/def/DefClusterInvokerTest.java b/trpc-core/src/test/java/com/tencent/trpc/core/cluster/def/DefClusterInvokerTest.java index ff258e51c..daff6cc22 100644 --- a/trpc-core/src/test/java/com/tencent/trpc/core/cluster/def/DefClusterInvokerTest.java +++ b/trpc-core/src/test/java/com/tencent/trpc/core/cluster/def/DefClusterInvokerTest.java @@ -33,12 +33,14 @@ import com.tencent.trpc.core.worker.handler.TrpcThreadExceptionHandler; import com.tencent.trpc.core.worker.spi.WorkerPool; import com.tencent.trpc.core.worker.support.thread.ThreadWorkerPool; +import java.lang.reflect.Field; import java.util.HashMap; import java.util.Map; import java.util.Objects; import java.util.concurrent.CompletableFuture; import java.util.concurrent.CompletionException; import java.util.concurrent.CompletionStage; +import java.util.concurrent.ConcurrentMap; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.atomic.AtomicLong; @@ -248,4 +250,55 @@ public void testDoInvokeWithCompletedFuture() { } } + @Test + @SuppressWarnings("unchecked") + public void testGetInvokerSkipsUnavailableCachedInvoker() throws Exception { + Assert.assertFalse(consumerInvokerProxy.isAvailable()); + + // Put the unavailable invoker into invokerCache via reflection + Field cacheField = DefClusterInvoker.class.getDeclaredField("invokerCache"); + cacheField.setAccessible(true); + ConcurrentMap> cache = + (ConcurrentMap>) cacheField.get(defClusterInvoker); + + ServiceInstance instance = new ServiceInstance("127.0.0.1", 12345); + String key = "127.0.0.1:12345:null"; + cache.put(key, consumerInvokerProxy); + + // Spy on defClusterInvoker and mock createInvoker to return a new available proxy + DefClusterInvoker spy = PowerMockito.spy(defClusterInvoker); + ConsumerInvokerProxy newProxy = PowerMockito.mock(ConsumerInvokerProxy.class); + PowerMockito.when(newProxy.isAvailable()).thenReturn(true); + PowerMockito.doReturn(newProxy).when(spy).createInvoker(instance); + + ConsumerInvokerProxy result = spy.getInvoker(instance); + + // Should not return the unavailable cached invoker + Assert.assertNotSame(consumerInvokerProxy, result); + // Should return the new available one from createInvoker + Assert.assertSame(newProxy, result); + } + + @Test + @SuppressWarnings("unchecked") + public void testGetInvokerReturnsAvailableCachedInvoker() throws Exception { + // Put an available invoker into invokerCache via reflection + Field cacheField = DefClusterInvoker.class.getDeclaredField("invokerCache"); + cacheField.setAccessible(true); + ConcurrentMap> cache = + (ConcurrentMap>) cacheField.get(defClusterInvoker); + + ServiceInstance instance = new ServiceInstance("127.0.0.1", 12345); + String key = "127.0.0.1:12345:null"; + + // Create an available proxy (client.isAvailable() = true) + ConsumerInvokerProxy availableProxy = PowerMockito.mock(ConsumerInvokerProxy.class); + PowerMockito.when(availableProxy.isAvailable()).thenReturn(true); + cache.put(key, availableProxy); + + // getInvoker should return the cached available proxy directly + ConsumerInvokerProxy result = defClusterInvoker.getInvoker(instance); + Assert.assertSame(availableProxy, result); + } + }