Today, when I was in the group codereview, I was found out by the bosses that my program was written and shared. Some of the code involved internal secrets. I tried to describe it clearly with comments

scene

I'm a conversation scenario. I need to lock the group session and then reply automatically. That is, when a message comes in, I can't let everyone in the same group trigger my automatic reply rule. I can only lock / unlock it after I get the lock and perform automatic reply unlocking

code

  			//Judge the need for locking
            boolean lockFlag = judgeLock(chatMsgDO.getChatType(),autoReplyRuleDO);
            if (lockFlag){
                //Group chat message processing
                //Session ID + rule primary key ID
                String key = RedisKeyConstant.genAutoReplyKey(chatMsgDO.getBindId(),autoReplyRuleDO.getId());
                try{
                    redisService.lockWithExpireTimeAndRetryTimes(key, 60, 5);
                }catch (Exception e){
                    log.info("[Keyword automatic reply(Group chat)]Locking failed,{}Wechat does not process",chatMsgDO.getReceiverId());
                }
                //It is directly guaranteed that only one hosted wechat will automatically reply to the same message content
                //Judge whether to reply to group chat@
                 boolean mentionSwitch = judgeNeedMention(autoReplyRuleDO);
                 if (mentionSwitch == true) {
                      //Need @, splicing @ people can only carry text messages at present@
                      conversationSendMsgCommand.setMentionIdList(Arrays.asList(chatMsgDO.getSenderId()));
                 }
                 //Process auto reply common logic
                 handlerAutoReplyEvent(sendMsgBO, chatMsgDO, autoReplyRuleDO);
   				 redisService.unlock(key);
            }

problem

There is a problem in this place, because my project is in a cluster environment, not just a machine. Because the code I write is a little pulled and has bug s, I'll sort it out
1. Locking and unlocking are not in try, catch and finally (think about it, even if finally is added)
2. Without considering the scenario of locking failure, what happens to the locking failure program?

If I simply add try/catch/finally, do you think this is correct?

  			boolean lockFlag = judgeLock(chatMsgDO.getChatType(),autoReplyRuleDO);
            if (lockFlag){
                //Group chat message processing
                //Session ID + rule primary key ID
                String key = RedisKeyConstant.genAutoReplyKey(chatMsgDO.getBindId(),autoReplyRuleDO.getId());
                try{
                    redisService.lockWithExpireTimeAndRetryTimes(key, 60, 5);
                    //It is directly guaranteed that only one hosted wechat will automatically reply to the same message content
                    //Judge whether to reply to group chat@
                    boolean mentionSwitch = judgeNeedMention(autoReplyRuleDO);
                    if (mentionSwitch == true) {
                         //Need @, splicing @ people can only carry text messages at present@
                         conversationSendMsgCommand.setMentionIdList(Arrays.asList(chatMsgDO.getSenderId()));
                    }
                    //Process auto reply common logic
                    handlerAutoReplyEvent(sendMsgBO, chatMsgDO, autoReplyRuleDO);
                }catch (Exception e){
                    log.info("[Keyword automatic reply(Group chat)]Locking failed,{}Wechat does not process",chatMsgDO.getReceiverId());
                }finally {
                    //Last session message unlock
                    redisService.unlock(key);
                }          
            }

Is this version of the code safe?
The answer is not safe
1. All basic partners know that if an exception is thrown in a try, finally it will be executed, right? How can it be unlocked? This place will report an error
2. Even if the locking is successful, but we are in a cluster environment, will there be a thread of machine 2 that releases the newly added lock of machine 1? The answer is yes, it is possible

Let me draw a picture. You can understand it this way

terms of settlement

1. Code writing / process, so that it will not happen. That is to say, I can unlock only if I lock it successfully. I won't let you go finally if you fail. The code is as follows

  boolean lockFlag = judgeLock(chatMsgDO.getChatType(),autoReplyRuleDO);
            if (lockFlag){
                //Group chat message processing
                //Session ID + rule primary key ID
                String key = RedisKeyConstant.genAutoReplyKey(chatMsgDO.getBindId(),autoReplyRuleDO.getId());
                boolean lock = redisService.lockWithExpireTimeAndRetryTimes(key, 60, 5);
                if(lock) {
                     try{
                         //It is directly guaranteed that only one hosted wechat will automatically reply to the same message content
                         //Judge whether to reply to group chat@
                         boolean mentionSwitch = judgeNeedMention(autoReplyRuleDO);
                         if (mentionSwitch == true) {
                             //Need @, splicing @ people can only carry text messages at present@
                             conversationSendMsgCommand.setMentionIdList(Arrays.asList(chatMsgDO.getSenderId()));
                         }
                         //Process auto reply common logic
                         handlerAutoReplyEvent(sendMsgBO, chatMsgDO, autoReplyRuleDO);
                     } catch (Exception e){
                         log.info("[Keyword automatic reply(Group chat)]Locking failed,{}Wechat does not process",chatMsgDO.getReceiverId());
                     }finally {
                         //Last session message unlock
                         redisService.unlock(key);
                     }
                } else {
                    //Locking failed
                    log.error("[Locking failed],Message content={}",chatMsgDO);
                    return;
                }
            }

1. The second method is to carry a unique ID. when unlocking, judge whether the unique ID is the same, and only the same can be unlocked
   Common methods include thread number and UUID, which have stronger hashing
   Carry this random number when locking

//When locking
 String key = RedisKeyConstant.genAutoReplyKey(chatMsgDO.getBindId(),autoReplyRuleDO.getId(),random number/thread  ID);

//When unlocking
if(Unique identification==random number/thread  ID)
	 redisService.unlock(key);