Skip to content

MINOR: Move lock method outside try block#9687

Merged
chia7712 merged 1 commit into
apache:trunkfrom
g1geordie:MINOR-lock
Dec 4, 2020
Merged

MINOR: Move lock method outside try block#9687
chia7712 merged 1 commit into
apache:trunkfrom
g1geordie:MINOR-lock

Conversation

@g1geordie

Copy link
Copy Markdown
Contributor

The Lock.lock in the try block may cause Lock.unlock throw exception when it throw exception .
I think it's nice to move outside although ReentrantLock.lock impl doesn't throw exception.

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

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

pattern recommended by
https://docs.oracle.com/javase/10/docs/api/java/util/concurrent/locks/ReentrantLock.html

@g1geordie

Copy link
Copy Markdown
Contributor Author

@chia7712 can you help me to take a look?

@chia7712 chia7712 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@g1geordie LGTM. thanks for you patch. I will merge it tomorrow if no objection.

@chia7712
chia7712 merged commit cc0247b into apache:trunk Dec 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants