代码的意图是针对每个 lockKey 同一时刻只能有一个线程处理
private final Map<String, Object> lockMap = new ConcurrentHashMap<>(32);
...
synchronized (lockMap.computeIfAbsent(lockKey, key -> new Object())) {
try {
...
} catch(Exception e) {
...
} finaly {
lockMap.remove(lockKey);
}
}
1
moshiyeap100 2022-09-30 15:12:55 +08:00
1. 如果是微服务,多服务的情况下不可以。
|
2
wolfie 2022-09-30 15:14:37 +08:00 2
不能
A 执行完 remove key => B 执行中 => C computeIfAbsent 拿 new Object(); |
3
moshiyeap100 2022-09-30 15:14:39 +08:00
如果不是微服务,直接 synchronized(key.intern()){},虽然常量池可能会越来越大。
|
4
ipwx 2022-09-30 15:16:57 +08:00 1
这问题很大。因为你 lockMap 本身没有锁,所以你在拿到 lock 对象前的操作都有问题。
你这需求很早有人就做过了。比如 https://yanbin.blog/google-guava-striped-key-based-fine-grain-locks/ |
5
slomo 2022-09-30 15:17:34 +08:00
em.... 直接在 computeIfAbsent 的第二个参数, 也就是 Function 里做处理, 最后返回 null, 是不是也可行
|
6
ipwx 2022-09-30 15:17:52 +08:00 1
简单来说就是不需要每个 key 一个锁。给一个固定大小的锁池,把 key 哈希映射到锁池里面。这样既能一定程度上分散锁,又不用动态创建新的锁,锁的总数也是确定的。
|
7
momocraft 2022-09-30 15:19:35 +08:00
如果不 remove 就能用吧, computeIfAbsent 保证一个 lockKey 最多只会创建一次 lock object
|
9
0xcaffebabe OP 学习到了,Guava 的 Striped ,感谢各位
|
10
dqzcwxb 2022-09-30 15:34:45 +08:00
|
11
xiang0818 2022-09-30 16:16:39 +08:00
@moshiyeap100 这个和微服务有啥关系,这是单机 map
|
12
imaple 2022-09-30 16:25:25 +08:00
没问题啊
|
13
rqxiao 2022-09-30 16:32:46 +08:00
remove 为什么会线程不安全啊
|
14
hsymlg 2022-09-30 16:48:37 +08:00
这代码为什么会有线程安全问题啊🤔
|
15
zhulixin 2022-09-30 17:26:24 +08:00 6
remove 代表锁被移除了,此时 sync 中的 obj 还没有解锁。这时候其他线程 get 同样的 key ,就拿到了新锁。即同一个 Key 产生了多把锁。
|
16
7911364440 2022-09-30 17:39:05 +08:00
@zhulixin 如果把 remove 放在 synchronized 外面应该就没问题了吧
```java private final Map<String, Object> lockMap = new ConcurrentHashMap<>(32); ... try { synchronized (lockMap.computeIfAbsent(lockKey, key -> new Object())) { ... } } catch(Exception e) { ... } finaly { lockMap.remove(lockKey); } ``` |
17
cubecube 2022-09-30 17:42:44 +08:00
@dqzcwxb 一般情况下不要用 intern ,有严重的性能问题,也没啥收益。现在 GC 都有 String Deduplication 了
|
18
ainyyy 2022-09-30 18:24:07 +08:00
@7911364440 还是一样的问题吧,第一次锁释放 remove 前,第二个 key 获取到锁进入执行,执行过程中被释放,还是会生成新的锁对象
|
19
clownpiece 2022-09-30 20:01:18 +08:00
if (lockMap.computeIfAbsent(lockKey, k -> new AtomicBoolean()).compareAndSet(false, true)) {
try { // ... } finally { lockMap.remove(lockKey); } } |
20
leonshaw 2022-09-30 20:07:48 +08:00 via Android 1
从 map 里拿到对象到加锁中间有窗口,无法保证加锁时对象还在 map 里。
|
21
yhvictor 2022-09-30 21:23:45 +08:00 via iPhone
这操作太骚气了,得看源代码才知道。
但是重入不是等待而是直接丢弃,这样不太好吧。 |
22
gosidealone 2022-09-30 21:49:49 +08:00
这为什么有问题呢? remove 之后逻辑已经走完了,即使有相同的 key 进来也不会影响吧。
|
23
moshiyeap100 2022-09-30 22:38:22 +08:00
@xiang0818 我的意思是微服务多实例部署的情况下,单机锁是无效的,如果同一个 key 请求到两个服务上,实际上两个服务都会处理。
|
24
xiangyuecn 2022-09-30 22:42:10 +08:00
没一点卵用的,同一个 key 会出现并发的情况简直就是混乱无比:
- 线程 1 创建了 object ,线程 2 等待线程 1 ,这是没有争议的 - 线程 1 执行完了,线程 2 开始执行,此时新来一个线程 3 将会如何执行? - 线程 2 执行完后,它 remove 到底 remove 掉了谁的 object ?此时再新来一个线程 4 ,线程 3 和线程 4 又是怎么执行情况? |
25
xiangyuecn 2022-09-30 22:47:58 +08:00 1
synchronized (lockMap.computeIfAbsent(lockKey, key -> new Object()))
这句本身就是线程不安全😂,线程 1 辛辛苦苦创建了一个 object ,写入进去了,但还没来得及返回上锁,被线程 2 抢走做核酸了,虽然罕见,但实属大冤种😂 |
26
14104chk 2022-10-01 09:08:49 +08:00
lockMap.remove(lockKey); 之后,锁就没有了,这时候当前线程 a 更改的变量可能还没同步到主内存,
同时又有另一个线程 b 获取锁,b 从主内存读取数据,因为这时候线程 a 的数据没有同步回主内存,所以 b 读到的还是旧数据 如果要使这段代码是线程安全的,就要给涉及的变量加上 volatile ,让变量的更新直接在主内存进行 |
27
7911364440 2022-10-01 09:53:53 +08:00
@leonshaw 锁对象就算不在 map 里感觉也没有影响吧
|
28
fallingg 2022-10-01 12:09:30 +08:00 via iPhone
map 是 concurrentmap ,所以对同一个 key 只会有一个线程去执行 try block 中的语句。但#26 可能说的是对的,在 remove 后,对同一个 key 不同线程可能上锁的不是同一个对象,这时候线程 a 的数据对 b 来说可能不可见。这段代码里如果去掉 remove key 的语句应该就是线程安全了。但是这样的话,可能会出现 key 特别多的场景,内存上会有问题。所以如果 key 的数量是有限的话,去掉 remove 语句后可以用。
|
29
fallingg 2022-10-01 12:11:02 +08:00 via iPhone
不过即使 remove key ,map 因此而扩容的数组应该也没无法释放 长期可能会有内存泄漏的现象。所以楼上池化的想法也不错
|
30
tairan2006 2022-10-01 15:00:35 +08:00
我记得有个编码规范是不要在 mapping function 里面再次更新当前 map 啊…而且你用了并发安全的容器,再用锁不就性能更差了?
一个简单的方案:在外侧创建一个 uuid ,去掉`synchronized`,直接用`computeIfAbsent`放入 uuid ,后面用 removeIf 判断 value 是这个 uuid 的话,移除掉 key 就行了。 |