[jira] [Comment Edited] (KYLIN-3578) Do not synchronize on the intrinsic locks of high-level concurrency objects

["XiaoXiang Yu (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/KYLIN-3578?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16631400#comment-16631400 ] 

XiaoXiang Yu edited comment on KYLIN-3578 at 9/28/18 6:16 AM:
--------------------------------------------------------------

 
{quote}private final ReentrantLock lock = new ReentrantLock();{quote}
{quote}{{if (delta < 0) {}}
{{  synchronized (lock) {}}
{{    lock.notifyAll();}}
{{  }}}
{{}}}
{quote}
 

This code above seems violates good practice, the reporter says use intrinsic lock on high-level lock is a mistake, use tryLock method of high-level lock is correct.

But code above just regard _+lock+_ object as a normal object and use tryLock to replace it a real mistake. In my view, it looks like a trick other than a mistake.

So it is no need to repair it. It has no relation with intrinsic lock on high-level lock mistake.

[oracle locksync doc|https://docs.oracle.com/javase/tutorial/essential/concurrency/locksync.html]
[Chinese Javadoc|https://github.com/hit-lacus/kylin/blob/86108c41552dea1f248abb71ddbeac8f82900005/core-common/src/main/java/org/apache/kylin/common/util/MemoryBudgetController.java]
 [Do not synchronize on the intrinsic locks of high-level concurrency objects|https://wiki.sei.cmu.edu/confluence/display/java/LCK03-J.+Do+not+synchronize+on+the+intrinsic+locks+of+high-level+concurrency+objects]

 


was (Author: hit_lacus):
{quote}if (delta < 0) {
   synchronized (lock) {
      lock.notifyAll();
   }
}{quote}
This code above seems violates good practice, but it's just regard _+lock+_ object as a normal object. It looks like a trick other than a mistake.

So it is no need to repair it. It has no relation with intrinsic lock on high-level lock.

[oracle locksync doc|https://docs.oracle.com/javase/tutorial/essential/concurrency/locksync.html]
[Chinese Javadoc|https://github.com/hit-lacus/kylin/blob/86108c41552dea1f248abb71ddbeac8f82900005/core-common/src/main/java/org/apache/kylin/common/util/MemoryBudgetController.java]
[Do not synchronize on the intrinsic locks of high-level concurrency objects|https://wiki.sei.cmu.edu/confluence/display/java/LCK03-J.+Do+not+synchronize+on+the+intrinsic+locks+of+high-level+concurrency+objects]

 

 

> Do not synchronize on the intrinsic locks of high-level concurrency objects
> ---------------------------------------------------------------------------
>
>                 Key: KYLIN-3578
>                 URL: https://issues.apache.org/jira/browse/KYLIN-3578
>             Project: Kylin
>          Issue Type: Bug
>            Reporter: Ted Yu
>            Assignee: XiaoXiang Yu
>            Priority: Major
>             Fix For: v2.6.0
>
>
> From core-common/src/main/java/org/apache/kylin/common/util/MemoryBudgetController.java :
> {code}
>     private final ReentrantLock lock = new ReentrantLock();
> ...
>             synchronized (lock) {
> {code}
> See the following for why such practice is to be avoided:
> https://wiki.sei.cmu.edu/confluence/display/java/LCK03-J.+Do+not+synchronize+on+the+intrinsic+locks+of+high-level+concurrency+objects



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)