V2EX = way to explore
V2EX 是一个关于分享和探索的地方
现在注册
已注册用户请  登录
0xcaffebabe
V2EX  ›  Java

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

  •  3
     
  •   0xcaffebabe ·
    0xcaffebabe · 2022-09-30 15:00:50 +08:00 · 4568 次点击
    这是一个创建于 785 天前的主题,其中的信息可能已经有所发展或是发生改变。

    代码的意图是针对每个 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 条回复    2022-10-10 09:32:52 +08:00
    moshiyeap100
        1
    moshiyeap100  
       2022-09-30 15:12:55 +08:00
    1. 如果是微服务,多服务的情况下不可以。
    wolfie
        2
    wolfie  
       2022-09-30 15:14:37 +08:00   ❤️ 2
    不能

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

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

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

    ```
    cubecube
        17
    cubecube  
       2022-09-30 17:42:44 +08:00
    @dqzcwxb 一般情况下不要用 intern ,有严重的性能问题,也没啥收益。现在 GC 都有 String Deduplication 了
    ainyyy
        18
    ainyyy  
       2022-09-30 18:24:07 +08:00
    @7911364440 还是一样的问题吧,第一次锁释放 remove 前,第二个 key 获取到锁进入执行,执行过程中被释放,还是会生成新的锁对象
    clownpiece
        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);
    }
    }
    leonshaw
        20
    leonshaw  
       2022-09-30 20:07:48 +08:00 via Android   ❤️ 1
    从 map 里拿到对象到加锁中间有窗口,无法保证加锁时对象还在 map 里。
    yhvictor
        21
    yhvictor  
       2022-09-30 21:23:45 +08:00 via iPhone
    这操作太骚气了,得看源代码才知道。
    但是重入不是等待而是直接丢弃,这样不太好吧。
    gosidealone
        22
    gosidealone  
       2022-09-30 21:49:49 +08:00
    这为什么有问题呢? remove 之后逻辑已经走完了,即使有相同的 key 进来也不会影响吧。
    moshiyeap100
        23
    moshiyeap100  
       2022-09-30 22:38:22 +08:00
    @xiang0818 我的意思是微服务多实例部署的情况下,单机锁是无效的,如果同一个 key 请求到两个服务上,实际上两个服务都会处理。
    xiangyuecn
        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 又是怎么执行情况?
    xiangyuecn
        25
    xiangyuecn  
       2022-09-30 22:47:58 +08:00   ❤️ 1
    synchronized (lockMap.computeIfAbsent(lockKey, key -> new Object()))

    这句本身就是线程不安全😂,线程 1 辛辛苦苦创建了一个 object ,写入进去了,但还没来得及返回上锁,被线程 2 抢走做核酸了,虽然罕见,但实属大冤种😂
    14104chk
        26
    14104chk  
       2022-10-01 09:08:49 +08:00
    lockMap.remove(lockKey); 之后,锁就没有了,这时候当前线程 a 更改的变量可能还没同步到主内存,
    同时又有另一个线程 b 获取锁,b 从主内存读取数据,因为这时候线程 a 的数据没有同步回主内存,所以 b 读到的还是旧数据
    如果要使这段代码是线程安全的,就要给涉及的变量加上 volatile ,让变量的更新直接在主内存进行
    7911364440
        27
    7911364440  
       2022-10-01 09:53:53 +08:00
    @leonshaw 锁对象就算不在 map 里感觉也没有影响吧
    fallingg
        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 语句后可以用。
    fallingg
        29
    fallingg  
       2022-10-01 12:11:02 +08:00 via iPhone
    不过即使 remove key ,map 因此而扩容的数组应该也没无法释放 长期可能会有内存泄漏的现象。所以楼上池化的想法也不错
    tairan2006
        30
    tairan2006  
       2022-10-01 15:00:35 +08:00
    我记得有个编码规范是不要在 mapping function 里面再次更新当前 map 啊…而且你用了并发安全的容器,再用锁不就性能更差了?

    一个简单的方案:在外侧创建一个 uuid ,去掉`synchronized`,直接用`computeIfAbsent`放入 uuid ,后面用 removeIf 判断 value 是这个 uuid 的话,移除掉 key 就行了。
    Aresxue
        31
    Aresxue  
       2022-10-08 10:54:26 +08:00
    @ipwx 这种是不是比较依赖 hash 算法,不然 hash 碰撞了不就 g 了吗?拿时间换空间?
    ipwx
        32
    ipwx  
       2022-10-10 09:32:52 +08:00
    @Aresxue 锁太多了更慢。
    关于   ·   帮助文档   ·   博客   ·   API   ·   FAQ   ·   实用小工具   ·   2660 人在线   最高记录 6679   ·     Select Language
    创意工作者们的社区
    World is powered by solitude
    VERSION: 3.9.8.5 · 26ms · UTC 10:22 · PVG 18:22 · LAX 02:22 · JFK 05:22
    Developed with CodeLauncher
    ♥ Do have faith in what you're doing.