-
-
Notifications
You must be signed in to change notification settings - Fork 9.1k
修复长时间运行时 Redis 命令中断导致 accessToken 刷新失败的问题 #3932
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -46,7 +46,7 @@ public void lock() { | |
| try { | ||
| Thread.sleep(1000); | ||
| } catch (InterruptedException e) { | ||
| // Ignore | ||
| Thread.currentThread().interrupt(); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| } | ||
| } | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,98 @@ | ||
| package me.chanjar.weixin.common.util.locks; | ||
|
|
||
| import org.mockito.Mockito; | ||
| import org.springframework.data.redis.core.StringRedisTemplate; | ||
| import org.springframework.data.redis.core.ValueOperations; | ||
| import org.testng.Assert; | ||
| import org.testng.annotations.BeforeMethod; | ||
| import org.testng.annotations.Test; | ||
|
|
||
| import java.util.concurrent.TimeUnit; | ||
| import java.util.concurrent.atomic.AtomicBoolean; | ||
|
|
||
| /** | ||
| * 测试 RedisTemplateSimpleDistributedLock 的线程中断处理行为 | ||
| * | ||
| * @author GitHub Copilot | ||
| */ | ||
| public class RedisTemplateSimpleDistributedLockInterruptTest { | ||
|
|
||
| private StringRedisTemplate mockRedisTemplate; | ||
| private ValueOperations<String, String> mockValueOps; | ||
| private RedisTemplateSimpleDistributedLock lock; | ||
|
|
||
| @BeforeMethod | ||
| @SuppressWarnings("unchecked") | ||
| public void setUp() { | ||
| mockRedisTemplate = Mockito.mock(StringRedisTemplate.class); | ||
| mockValueOps = Mockito.mock(ValueOperations.class); | ||
| Mockito.when(mockRedisTemplate.opsForValue()).thenReturn(mockValueOps); | ||
| lock = new RedisTemplateSimpleDistributedLock(mockRedisTemplate, "test_interrupt_lock", 60000); | ||
| } | ||
|
|
||
| /** | ||
| * 测试 lock() 在 Thread.sleep 被中断时应恢复线程中断标志 | ||
| * <p> | ||
| * 修复前:InterruptedException 被忽略(// Ignore),线程中断标志丢失 | ||
| * 修复后:调用 Thread.currentThread().interrupt() 恢复中断标志 | ||
| * </p> | ||
| */ | ||
| @Test(description = "lock() 方法在中断时应恢复线程中断标志") | ||
| public void testLockRestoresInterruptedFlagAfterSleepInterruption() throws InterruptedException { | ||
| AtomicBoolean interruptedFlagAfterLock = new AtomicBoolean(false); | ||
|
|
||
| // 第一次 setIfAbsent 返回 false(模拟锁被占用),第二次返回 true(模拟锁释放) | ||
| Mockito.when(mockValueOps.setIfAbsent(Mockito.anyString(), Mockito.anyString(), | ||
| Mockito.anyLong(), Mockito.any(TimeUnit.class))) | ||
| .thenReturn(false) | ||
| .thenReturn(true); | ||
| // get() 返回不同的值,确保不走可重入路径 | ||
| Mockito.when(mockValueOps.get(Mockito.anyString())).thenReturn("other-value"); | ||
|
|
||
| Thread testThread = new Thread(() -> { | ||
| // 设置中断标志 | ||
| Thread.currentThread().interrupt(); | ||
| // 调用 lock(),第一次 tryLock 失败,sleep 会因中断标志立即抛出 InterruptedException | ||
| lock.lock(); | ||
| interruptedFlagAfterLock.set(Thread.currentThread().isInterrupted()); | ||
| }); | ||
|
|
||
| testThread.start(); | ||
| testThread.join(5000); | ||
|
|
||
| // 线程应该已经完成(不会永远阻塞) | ||
| Assert.assertFalse(testThread.isAlive(), "线程应该已完成"); | ||
| // 关键验证:中断标志应被恢复(而非被忽略丢失) | ||
| Assert.assertTrue(interruptedFlagAfterLock.get(), "lock()执行后线程中断标志应被恢复"); | ||
| } | ||
|
|
||
| /** | ||
| * 测试 tryLock() 在 Redis 正常响应时的基本行为 | ||
| */ | ||
| @Test(description = "tryLock() 成功获取锁时应返回 true") | ||
| public void testTryLockSuccessfully() { | ||
| Mockito.when(mockValueOps.setIfAbsent(Mockito.anyString(), Mockito.anyString(), | ||
| Mockito.anyLong(), Mockito.any(TimeUnit.class))) | ||
| .thenReturn(true); | ||
|
|
||
| boolean result = lock.tryLock(); | ||
|
|
||
| Assert.assertTrue(result, "应成功获取锁"); | ||
| Assert.assertNotNull(lock.getLockSecretValue(), "锁值不应为null"); | ||
| } | ||
|
|
||
| /** | ||
| * 测试 tryLock() 在锁已被其他线程持有时应返回 false | ||
| */ | ||
| @Test(description = "锁被占用时 tryLock() 应返回 false") | ||
| public void testTryLockWhenLockHeld() { | ||
| Mockito.when(mockValueOps.setIfAbsent(Mockito.anyString(), Mockito.anyString(), | ||
| Mockito.anyLong(), Mockito.any(TimeUnit.class))) | ||
| .thenReturn(false); | ||
| Mockito.when(mockValueOps.get(Mockito.anyString())).thenReturn("other-lock-value"); | ||
|
|
||
| boolean result = lock.tryLock(); | ||
|
|
||
| Assert.assertFalse(result, "锁被占用时应返回false"); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,7 @@ | ||
| package me.chanjar.weixin.cp.config.impl; | ||
|
|
||
| import lombok.NonNull; | ||
| import lombok.extern.slf4j.Slf4j; | ||
| import me.chanjar.weixin.common.bean.WxAccessToken; | ||
| import me.chanjar.weixin.common.redis.WxRedisOps; | ||
| import org.apache.commons.lang3.StringUtils; | ||
|
|
@@ -12,6 +13,7 @@ | |
| * @author yl | ||
| * created on 2023/04/23 | ||
| */ | ||
| @Slf4j | ||
| public abstract class AbstractWxCpInRedisConfigImpl extends WxCpDefaultConfigImpl { | ||
| private static final long serialVersionUID = 7157341535439380615L; | ||
| /** | ||
|
|
@@ -120,8 +122,15 @@ public String getAccessToken() { | |
|
|
||
| @Override | ||
| public boolean isAccessTokenExpired() { | ||
| Long expire = redisOps.getExpire(this.accessTokenKey); | ||
| return expire == null || expire < 2; | ||
| try { | ||
| Long expire = redisOps.getExpire(this.accessTokenKey); | ||
| return expire == null || expire < 2; | ||
| } catch (Exception e) { | ||
| log.warn("获取access_token过期时间时发生异常,将视为已过期以触发刷新,异常信息: {}", e.getMessage()); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Severity: low Other Locations
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage. |
||
| // 清除中断标志,确保后续的锁获取和token刷新操作能够正常执行 | ||
| Thread.interrupted(); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 这里无条件调用 Severity: medium Other Locations
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage. |
||
| return true; | ||
| } | ||
|
Comment on lines
124
to
+133
|
||
| } | ||
|
|
||
| @Override | ||
|
|
@@ -146,8 +155,14 @@ public String getJsapiTicket() { | |
|
|
||
| @Override | ||
| public boolean isJsapiTicketExpired() { | ||
| Long expire = redisOps.getExpire(this.jsapiTicketKey); | ||
| return expire == null || expire < 2; | ||
| try { | ||
| Long expire = redisOps.getExpire(this.jsapiTicketKey); | ||
| return expire == null || expire < 2; | ||
| } catch (Exception e) { | ||
| log.warn("获取jsapi_ticket过期时间时发生异常,将视为已过期,异常信息: {}", e.getMessage()); | ||
| Thread.interrupted(); | ||
| return true; | ||
| } | ||
|
Comment on lines
157
to
+165
|
||
| } | ||
|
|
||
| @Override | ||
|
|
@@ -177,8 +192,14 @@ public String getAgentJsapiTicket() { | |
|
|
||
| @Override | ||
| public boolean isAgentJsapiTicketExpired() { | ||
| Long expire = redisOps.getExpire(this.agentJsapiTicketKey); | ||
| return expire == null || expire < 2; | ||
| try { | ||
| Long expire = redisOps.getExpire(this.agentJsapiTicketKey); | ||
| return expire == null || expire < 2; | ||
| } catch (Exception e) { | ||
| log.warn("获取agent_jsapi_ticket过期时间时发生异常,将视为已过期,异常信息: {}", e.getMessage()); | ||
| Thread.interrupted(); | ||
|
|
||
| return true; | ||
| } | ||
| } | ||
|
|
||
| @Override | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,132 @@ | ||
| package me.chanjar.weixin.cp.config.impl; | ||
|
|
||
| import me.chanjar.weixin.common.redis.WxRedisOps; | ||
| import org.mockito.Mockito; | ||
| import org.testng.Assert; | ||
| import org.testng.annotations.BeforeMethod; | ||
| import org.testng.annotations.Test; | ||
|
|
||
| import java.util.concurrent.locks.Lock; | ||
| import java.util.concurrent.locks.ReentrantLock; | ||
|
|
||
| /** | ||
| * 测试 AbstractWxCpInRedisConfigImpl 对 Redis 异常的容错处理 | ||
| * | ||
| * @author GitHub Copilot | ||
| */ | ||
| public class AbstractWxCpInRedisConfigImplTest { | ||
|
|
||
| private WxRedisOps mockRedisOps; | ||
| private AbstractWxCpInRedisConfigImpl config; | ||
|
|
||
| @BeforeMethod | ||
| public void setUp() { | ||
| mockRedisOps = Mockito.mock(WxRedisOps.class); | ||
| Mockito.when(mockRedisOps.getLock(Mockito.anyString())) | ||
| .thenReturn(new ReentrantLock()); | ||
|
|
||
| config = new AbstractWxCpInRedisConfigImpl(mockRedisOps, "test") { | ||
| // 使用匿名类提供具体实现用于测试 | ||
| }; | ||
| config.setCorpId("testCorpId"); | ||
| config.setAgentId(1); | ||
| } | ||
|
|
||
| /** | ||
| * 测试当 Redis getExpire 抛出异常时,isAccessTokenExpired() 应返回 true(视为已过期) | ||
| */ | ||
| @Test | ||
| public void testIsAccessTokenExpiredWhenRedisThrowsException() { | ||
| Mockito.when(mockRedisOps.getExpire(Mockito.anyString())) | ||
| .thenThrow(new RuntimeException("Redis command interrupted")); | ||
|
|
||
| boolean expired = config.isAccessTokenExpired(); | ||
|
|
||
| Assert.assertTrue(expired, "Redis异常时应将token视为已过期"); | ||
| Assert.assertFalse(Thread.currentThread().isInterrupted(), "处理异常后线程中断标志应被清除"); | ||
| } | ||
|
|
||
| /** | ||
| * 测试当线程中断状态已设置时,Redis 调用抛出异常,isAccessTokenExpired() 应处理并清除中断标志 | ||
| */ | ||
| @Test | ||
| public void testIsAccessTokenExpiredClearsInterruptedFlag() { | ||
| Mockito.when(mockRedisOps.getExpire(Mockito.anyString())) | ||
| .thenThrow(new RuntimeException("Redis command interrupted")); | ||
|
|
||
| // 设置线程中断标志 | ||
| Thread.currentThread().interrupt(); | ||
|
|
||
| boolean expired = config.isAccessTokenExpired(); | ||
|
|
||
| Assert.assertTrue(expired, "Redis异常时应将token视为已过期"); | ||
| // 中断标志应该被清除,允许后续操作正常进行 | ||
| Assert.assertFalse(Thread.currentThread().isInterrupted(), "处理异常后线程中断标志应被清除"); | ||
|
Comment on lines
+59
to
+64
|
||
| } | ||
|
|
||
| /** | ||
| * 测试正常情况下 isAccessTokenExpired() 的行为 | ||
| */ | ||
| @Test | ||
| public void testIsAccessTokenExpiredWhenTokenValid() { | ||
| // 返回60秒后过期(未过期) | ||
| Mockito.when(mockRedisOps.getExpire(Mockito.anyString())).thenReturn(60L); | ||
|
|
||
| boolean expired = config.isAccessTokenExpired(); | ||
|
|
||
| Assert.assertFalse(expired, "token未过期时应返回false"); | ||
| } | ||
|
|
||
| /** | ||
| * 测试 isAccessTokenExpired() 当 expire 为 null 时视为已过期 | ||
| */ | ||
| @Test | ||
| public void testIsAccessTokenExpiredWhenExpireIsNull() { | ||
| Mockito.when(mockRedisOps.getExpire(Mockito.anyString())).thenReturn(null); | ||
|
|
||
| boolean expired = config.isAccessTokenExpired(); | ||
|
|
||
| Assert.assertTrue(expired, "expire为null时应视为已过期"); | ||
| } | ||
|
|
||
| /** | ||
| * 测试当 Redis getExpire 抛出异常时,isJsapiTicketExpired() 应返回 true(视为已过期) | ||
| */ | ||
| @Test | ||
| public void testIsJsapiTicketExpiredWhenRedisThrowsException() { | ||
| Mockito.when(mockRedisOps.getExpire(Mockito.anyString())) | ||
| .thenThrow(new RuntimeException("Redis command interrupted")); | ||
|
|
||
| boolean expired = config.isJsapiTicketExpired(); | ||
|
|
||
| Assert.assertTrue(expired, "Redis异常时应将jsapi_ticket视为已过期"); | ||
| Assert.assertFalse(Thread.currentThread().isInterrupted(), "处理异常后线程中断标志应被清除"); | ||
| } | ||
|
|
||
| /** | ||
| * 测试当 Redis getExpire 抛出异常时,isAgentJsapiTicketExpired() 应返回 true(视为已过期) | ||
| */ | ||
| @Test | ||
| public void testIsAgentJsapiTicketExpiredWhenRedisThrowsException() { | ||
| Mockito.when(mockRedisOps.getExpire(Mockito.anyString())) | ||
| .thenThrow(new RuntimeException("Redis command interrupted")); | ||
|
|
||
| boolean expired = config.isAgentJsapiTicketExpired(); | ||
|
|
||
| Assert.assertTrue(expired, "Redis异常时应将agent_jsapi_ticket视为已过期"); | ||
| Assert.assertFalse(Thread.currentThread().isInterrupted(), "处理异常后线程中断标志应被清除"); | ||
| } | ||
|
|
||
| /** | ||
| * 测试提供自定义 Lock 实现时 getAccessTokenLock() 返回正确的锁 | ||
| */ | ||
| @Test | ||
| public void testGetAccessTokenLockReturnsMockedLock() { | ||
| Lock mockLock = Mockito.mock(Lock.class); | ||
| Mockito.when(mockRedisOps.getLock(Mockito.anyString())).thenReturn(mockLock); | ||
|
|
||
| Lock lock = config.getAccessTokenLock(); | ||
|
|
||
| Assert.assertNotNull(lock, "获取到的锁不应为null"); | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When
lock()is waiting andThread.sleepis interrupted, setting the interrupt flag immediately inside the loop causes the next iteration to run withThread.currentThread().isInterrupted()==true. In production with Lettuce/Spring Redis, subsequenttryLock()Redis calls can then fail immediately withRedisCommandInterruptedException(or loop without effective sleeping), so a single interrupt while contending for the lock can make lock acquisition fail continuously instead of retrying normally.Useful? React with 👍 / 👎.