0xcaffebabe
V2EX  ›  Java

请问这段 Java 代码能保证线程安全吗

  •  3
     
  •   0xcaffebabe ·
    0xcaffebabe · Sep 30, 2022 · 6239 views
    This topic created in 1495 days ago, the information mentioned may be changed or developed.

    代码的意图是针对每个 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);
        }
    }
    
    32 replies  •  2022-10-10 09:32:52 +08:00
    moshiyeap100
    moshiyeap100
    Sep 30, 2022
    1
    1. 如果是微服务,多服务的情况下不可以。
    wolfie
    wolfie
    Sep 30, 2022❤️2
    2
    不能

    A 执行完 remove key => B 执行中 => C computeIfAbsent 拿 new Object();
    moshiyeap100
    moshiyeap100
    Sep 30, 2022
    3
    如果不是微服务,直接 synchronized(key.intern()){},虽然常量池可能会越来越大。
    ipwx
    ipwx
    Sep 30, 2022❤️1
    4
    这问题很大。因为你 lockMap 本身没有锁,所以你在拿到 lock 对象前的操作都有问题。

    你这需求很早有人就做过了。比如 https://yanbin.blog/google-guava-striped-key-based-fine-grain-locks/
    slomo
    slomo
    Sep 30, 2022
    5
    em.... 直接在 computeIfAbsent 的第二个参数, 也就是 Function 里做处理, 最后返回 null, 是不是也可行
    ipwx
    ipwx
    Sep 30, 2022❤️1
    6
    简单来说就是不需要每个 key 一个锁。给一个固定大小的锁池,把 key 哈希映射到锁池里面。这样既能一定程度上分散锁,又不用动态创建新的锁,锁的总数也是确定的。
    momocraft
    momocraft
    Sep 30, 2022
    7
    如果不 remove 就能用吧, computeIfAbsent 保证一个 lockKey 最多只会创建一次 lock object
    ipwx
    ipwx
    Sep 30, 2022❤️1
    8
    @momocraft 虽然你说得对,但是 key 一多就内存爆炸了。

    还是固定大小的锁池比较合理。
    0xcaffebabe
    0xcaffebabe
    OP
    Sep 30, 2022
    9
    学习到了,Guava 的 Striped ,感谢各位
    dqzcwxb
    dqzcwxb
    Sep 30, 2022
    10
    xiang0818
    xiang0818
    Sep 30, 2022
    11
    @moshiyeap100 这个和微服务有啥关系,这是单机 map
    imaple
    imaple
    Sep 30, 2022
    12
    没问题啊
    rqxiao
    rqxiao
    Sep 30, 2022
    13
    remove 为什么会线程不安全啊
    hsymlg
    hsymlg
    Sep 30, 2022
    14
    这代码为什么会有线程安全问题啊🤔
    rev0
    rev0
    Sep 30, 2022❤️6
    15
    remove 代表锁被移除了,此时 sync 中的 obj 还没有解锁。这时候其他线程 get 同样的 key ,就拿到了新锁。即同一个 Key 产生了多把锁。
    7911364440
    7911364440
    Sep 30, 2022
    16
    @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);
    }

    ```
    cubecube
    cubecube
    Sep 30, 2022
    17
    @dqzcwxb 一般情况下不要用 intern ,有严重的性能问题,也没啥收益。现在 GC 都有 String Deduplication 了
    ainyyy
    ainyyy
    Sep 30, 2022
    18
    @7911364440 还是一样的问题吧,第一次锁释放 remove 前,第二个 key 获取到锁进入执行,执行过程中被释放,还是会生成新的锁对象
    clownpiece
    clownpiece
    Sep 30, 2022
    19
    if (lockMap.computeIfAbsent(lockKey, k -> new AtomicBoolean()).compareAndSet(false, true)) {
    try {
    // ...
    } finally {
    lockMap.remove(lockKey);
    }
    }
    leonshaw
    leonshaw
    Sep 30, 2022 via Android❤️1
    20
    从 map 里拿到对象到加锁中间有窗口,无法保证加锁时对象还在 map 里。
    yhvictor
    yhvictor
    Sep 30, 2022 via iPhone
    21
    这操作太骚气了,得看源代码才知道。
    但是重入不是等待而是直接丢弃,这样不太好吧。
    gosidealone
    gosidealone
    Sep 30, 2022
    22
    这为什么有问题呢? remove 之后逻辑已经走完了,即使有相同的 key 进来也不会影响吧。
    moshiyeap100
    moshiyeap100
    Sep 30, 2022
    23
    @xiang0818 我的意思是微服务多实例部署的情况下,单机锁是无效的,如果同一个 key 请求到两个服务上,实际上两个服务都会处理。
    xiangyuecn
    xiangyuecn
    Sep 30, 2022
    24
    没一点卵用的,同一个 key 会出现并发的情况简直就是混乱无比:

    - 线程 1 创建了 object ,线程 2 等待线程 1 ,这是没有争议的

    - 线程 1 执行完了,线程 2 开始执行,此时新来一个线程 3 将会如何执行?

    - 线程 2 执行完后,它 remove 到底 remove 掉了谁的 object ?此时再新来一个线程 4 ,线程 3 和线程 4 又是怎么执行情况?
    xiangyuecn
    xiangyuecn
    Sep 30, 2022❤️1
    25
    synchronized (lockMap.computeIfAbsent(lockKey, key -> new Object()))

    这句本身就是线程不安全😂,线程 1 辛辛苦苦创建了一个 object ,写入进去了,但还没来得及返回上锁,被线程 2 抢走做核酸了,虽然罕见,但实属大冤种😂
    14104chk
    14104chk
    Oct 1, 2022
    26
    lockMap.remove(lockKey); 之后,锁就没有了,这时候当前线程 a 更改的变量可能还没同步到主内存,
    同时又有另一个线程 b 获取锁,b 从主内存读取数据,因为这时候线程 a 的数据没有同步回主内存,所以 b 读到的还是旧数据
    如果要使这段代码是线程安全的,就要给涉及的变量加上 volatile ,让变量的更新直接在主内存进行
    7911364440
    7911364440
    Oct 1, 2022
    27
    @leonshaw 锁对象就算不在 map 里感觉也没有影响吧
    fallingg
    fallingg
    Oct 1, 2022 via iPhone
    28
    map 是 concurrentmap ,所以对同一个 key 只会有一个线程去执行 try block 中的语句。但#26 可能说的是对的,在 remove 后,对同一个 key 不同线程可能上锁的不是同一个对象,这时候线程 a 的数据对 b 来说可能不可见。这段代码里如果去掉 remove key 的语句应该就是线程安全了。但是这样的话,可能会出现 key 特别多的场景,内存上会有问题。所以如果 key 的数量是有限的话,去掉 remove 语句后可以用。
    fallingg
    fallingg
    Oct 1, 2022 via iPhone
    29
    不过即使 remove key ,map 因此而扩容的数组应该也没无法释放 长期可能会有内存泄漏的现象。所以楼上池化的想法也不错
    keenkiller
    keenkiller
    Oct 1, 2022
    30
    我记得有个编码规范是不要在 mapping function 里面再次更新当前 map 啊…而且你用了并发安全的容器,再用锁不就性能更差了?

    一个简单的方案:在外侧创建一个 uuid ,去掉`synchronized`,直接用`computeIfAbsent`放入 uuid ,后面用 removeIf 判断 value 是这个 uuid 的话,移除掉 key 就行了。
    Aresxue
    Aresxue
    Oct 8, 2022
    31
    @ipwx 这种是不是比较依赖 hash 算法,不然 hash 碰撞了不就 g 了吗?拿时间换空间?
    ipwx
    ipwx
    Oct 10, 2022
    32
    @Aresxue 锁太多了更慢。
    About   ·   Help   ·   Advertise   ·   Blog   ·   API   ·   FAQ   ·   Privacy   ·   Solana   ·   5063 Online   Highest 6679   ·     Select Language
    创意工作者们的社区
    World is powered by solitude
    VERSION: 3.9.8.5 · 118ms · UTC 03:48 · PVG 11:48 · LAX 19:48 · JFK 22:48
    ♥ Do have faith in what you're doing.