Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ public void lock() {
try {
Thread.sleep(1000);
} catch (InterruptedException e) {
// Ignore
Thread.currentThread().interrupt();

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Avoid re-interrupting inside lock retry loop

When lock() is waiting and Thread.sleep is interrupted, setting the interrupt flag immediately inside the loop causes the next iteration to run with Thread.currentThread().isInterrupted()==true. In production with Lettuce/Spring Redis, subsequent tryLock() Redis calls can then fail immediately with RedisCommandInterruptedException (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 👍 / 👎.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里在捕获 InterruptedException 后立即重新设置中断标志,会导致后续 Thread.sleep 反复“立刻抛出”,在锁长时间不可用时可能形成忙等/高 CPU 循环并频繁触发 tryLock() 访问 Redis。建议确认 lock() 在中断场景下的期望语义(继续等待但避免自旋 vs 直接放弃等待)。

Severity: medium

Fix This in Augment

🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.

}
}
}
Expand Down
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;
Expand All @@ -12,6 +13,7 @@
* @author yl
* created on 2023/04/23
*/
@Slf4j
public abstract class AbstractWxCpInRedisConfigImpl extends WxCpDefaultConfigImpl {
private static final long serialVersionUID = 7157341535439380615L;
/**
Expand Down Expand Up @@ -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());
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

log.warn 目前只记录了 e.getMessage(),会丢失堆栈与根因类型信息,后续排查 Redis 中断/连接问题可能不够。建议确认是否需要保留异常对象信息以便定位。

Severity: low

Other Locations
  • weixin-java-cp/src/main/java/me/chanjar/weixin/cp/config/impl/AbstractWxCpInRedisConfigImpl.java:162
  • weixin-java-cp/src/main/java/me/chanjar/weixin/cp/config/impl/AbstractWxCpInRedisConfigImpl.java:199

Fix This in Augment

🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.

// 清除中断标志,确保后续的锁获取和token刷新操作能够正常执行
Thread.interrupted();
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里无条件调用 Thread.interrupted() 会清除调用方设置的中断信号(例如任务取消/应用关闭),即使异常并非由中断导致也会吞掉中断语义;建议至少仅在确认当前线程已被中断或异常链明确为“命令被中断”时再清除。

Severity: medium

Other Locations
  • weixin-java-cp/src/main/java/me/chanjar/weixin/cp/config/impl/AbstractWxCpInRedisConfigImpl.java:163
  • weixin-java-cp/src/main/java/me/chanjar/weixin/cp/config/impl/AbstractWxCpInRedisConfigImpl.java:200

Fix This in Augment

🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.

return true;
}
Comment on lines 124 to +133
}

@Override
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
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");
}
}
Loading