说下这段代码问题
class Account {
public int accountId;
public long balance;
}
class BankService {
public void transfer(Account a, Account b, long amount) {
synchronized(a) {
synchronized(b) {
if (a.balance < amount) {
throw new RuntimeException("balance not enough!");
}
b.balance += amount;
a.balance -= amount;
}
}
}
}
这段代码存在多个问题,主要集中在并发安全和设计缺陷上:
1. 死锁风险(最严重问题)
synchronized 嵌套锁定两个账户对象(a 和 b)时,未定义锁的获取顺序。若线程1执行 transfer(A, B, amount) 的同时线程2执行 transfer(B, A, amount):
- 线程1锁定
A后尝试锁定B - 线程2锁定
B后尝试锁定A
此时双方互相等待对方释放锁,导致死锁。
2. 封装性破坏
Account 的字段 accountId 和 balance 均为 public,允许外部直接修改:
javaCopy Code
a.balance = -100; // 非法操作,但代码未限制
应改为 private 字段并通过方法控制访问。
3. 余额检查后未二次验证
在检查 a.balance < amount 后直接修改余额。若检查后、扣款前其他线程修改了 a.balance(例如通过其他转账),余额可能被透支。需在锁内重新验证条件。
4. 数值溢出风险
balance 使用 long 类型,但未处理加减操作的溢出:
javaCopy Code
a.balance -= amount; // 若 amount 为负数或极大值,可能溢出
5. 异常设计不当
抛出 RuntimeException 过于笼统,调用方难以针对性处理。应自定义业务异常(如 InsufficientFundsException)。
6. 未校验金额合法性
amount 未检查是否为负数:
javaCopy Code
transfer(a, b, -100); // 导致 a 余额增加,b 余额减少
需添加 if (amount <= 0) throw ...。
改进建议
class Account {
private final int accountId; // 字段私有化
private long balance;
public Account(int accountId, long balance) {
this.accountId = accountId;
this.balance = balance;
}
// 封装余额修改逻辑
public void addBalance(long delta) {
this.balance = Math.addExact(balance, delta); // 防溢出
}
public long getBalance() { return balance; }
}
class BankService {
public void transfer(Account a, Account b, long amount) {
if (amount <= 0) throw new IllegalArgumentException("金额必须为正数");
// 按固定顺序加锁(避免死锁)
Account first = a.getAccountId() < b.getAccountId() ? a : b;
Account second = first == a ? b : a;
synchronized (first) {
synchronized (second) {
if (a.getBalance() < amount) {
throw new InsufficientFundsException("余额不足");
}
a.addBalance(-amount); // 原子操作
b.addBalance(amount);
}
}
}
}
关键改进:
- 通过账户ID排序锁对象解决死锁问题;
- 封装账户字段并校验数值边界;
- 使用
Math.addExact防止溢出; - 自定义异常提升可维护性。