rocketmq icon indicating copy to clipboard operation
rocketmq copied to clipboard

lock.lock() before try

Open shenjianeng opened this issue 1 year ago • 5 comments

lock.lock() before try

shenjianeng avatar Nov 26 '23 05:11 shenjianeng

在外面没什么问题。参考:https://juejin.cn/post/7269962653795270671

原写法是写到里面了,这个pr是改到外面的

joeCarf avatar Dec 01 '23 08:12 joeCarf

在外面没什么问题。参考:https://juejin.cn/post/7269962653795270671

这个回复好像容易误导人。

看下 jdk 的官方文档建议吧,应该来说是最“正宗的”,https://docs.oracle.com/javase/8/docs/api/java/util/concurrent/locks/ReentrantLock.html

It is recommended practice to always immediately follow a call to lock with a try block, most typically in a before/after construction such as:

 class X {
   private final ReentrantLock lock = new ReentrantLock();
   // ...

   public void m() {
     lock.lock();  // block until condition holds
     try {
       // ... method body
     } finally {
       lock.unlock()
     }
   }
 }

另外,上面掘金文章结尾有几句话:

  1. 最好是把 lock.lock() 加锁方法写到 try 外面,是一种规范,而不是强制。

  2. 如果你非要写到 try 里面,那么写到 try 语句块的第一行,或者 lock 加锁方法前面不会存在可能出现异常的代码。

  3. 最后,如果你代码中加锁放到了 try 语句里,麻烦参考第 1 点

严格来讲这几个结论,可能在 ReentrantLock 这个实现下是没问题的,但 lock 是接口,会有任意实现多种实现。

在不同的实现下,lock.lock() 方法抛出异常是很有可能的,因此不能放在 try 当中。

shenjianeng avatar Dec 01 '23 08:12 shenjianeng

更标准做法,是要放到try外面。

fuyou001 avatar Dec 18 '23 00:12 fuyou001

resolved conflicts

shenjianeng avatar Jan 29 '24 01:01 shenjianeng

Codecov Report

Attention: 10 lines in your changes are missing coverage. Please review.

Comparison is base (e104273) 43.25% compared to head (17259ae) 43.26%. Report is 2 commits behind head on develop.

Files Patch % Lines
...he/rocketmq/client/impl/consumer/ProcessQueue.java 61.11% 5 Missing and 2 partials :warning:
...tmq/namesrv/processor/DefaultRequestProcessor.java 77.77% 1 Missing and 1 partial :warning:
...a/org/apache/rocketmq/client/impl/MQAdminImpl.java 0.00% 1 Missing :warning:
Additional details and impacted files
@@              Coverage Diff              @@
##             develop    #7582      +/-   ##
=============================================
+ Coverage      43.25%   43.26%   +0.01%     
- Complexity      9880     9884       +4     
=============================================
  Files           1173     1173              
  Lines          85047    85049       +2     
  Branches       11014    11014              
=============================================
+ Hits           36788    36799      +11     
+ Misses         43692    43685       -7     
+ Partials        4567     4565       -2     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov-commenter avatar Jan 29 '24 02:01 codecov-commenter