请问这段 Java 代码能保证线程安全吗 - V2EX
V2EX = way to explore
V2EX 是一个关于分享和探索的地方
现在注册
已注册用户请  登录
0xcaffebabe
V2EX    Java

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

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

    代码的意图是针对每个 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
    这代码为什么会有线程安全问题啊
    rev0
        15
    rev0  
       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     Solana     878 人在线   最高记录 6679       Select Language
    创意工作者们的社区
    World is powered by solitude
    VERSION: 3.9.8.5 24ms UTC 19:49 PVG 03:49 LAX 12:49 JFK 15:49
    Do have faith in what you're doing.
    ubao snddm index pchome yahoo rakuten mypaper meadowduck bidyahoo youbao zxmzxm asda bnvcg cvbfg dfscv mmhjk xxddc yybgb zznbn ccubao uaitu acv GXCV ET GDG YH FG BCVB FJFH CBRE CBC GDG ET54 WRWR RWER WREW WRWER RWER SDG EW SF DSFSF fbbs ubao fhd dfg ewr dg df ewwr ewwr et ruyut utut dfg fgd gdfgt etg dfgt dfgd ert4 gd fgg wr 235 wer3 we vsdf sdf gdf ert xcv sdf rwer hfd dfg cvb rwf afb dfh jgh bmn lgh rty gfds cxv xcv xcs vdas fdf fgd cv sdf tert sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf shasha9178 shasha9178 shasha9178 shasha9178 shasha9178 liflif2 liflif2 liflif2 liflif2 liflif2 liblib3 liblib3 liblib3 liblib3 liblib3 zhazha444 zhazha444 zhazha444 zhazha444 zhazha444 dende5 dende denden denden2 denden21 fenfen9 fenf619 fen619 fenfe9 fe619 sdf sdf sdf sdf sdf zhazh90 zhazh0 zhaa50 zha90 zh590 zho zhoz zhozh zhozho zhozho2 lislis lls95 lili95 lils5 liss9 sdf0ty987 sdft876 sdft9876 sdf09876 sd0t9876 sdf0ty98 sdf0976 sdf0ty986 sdf0ty96 sdf0t76 sdf0876 df0ty98 sf0t876 sd0ty76 sdy76 sdf76 sdf0t76 sdf0ty9 sdf0ty98 sdf0ty987 sdf0ty98 sdf6676 sdf876 sd876 sd876 sdf6 sdf6 sdf9876 sdf0t sdf06 sdf0ty9776 sdf0ty9776 sdf0ty76 sdf8876 sdf0t sd6 sdf06 s688876 sd688 sdf86