[jira] [Comment Edited] (CURATOR-527) Concurrency issue in LockInternals

["Kim Jaechang (JIRA)" <ji...@apache.org>]

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

Kim Jaechang edited comment on CURATOR-527 at 6/5/19 4:28 PM:
--------------------------------------------------------------

[~randgalt] The case I said was node deleted after getData() called and before wait() called. 
I checked from thread dump that localvariable for lock-759(previousSequencePath) and lock-760(ourPath) exist but in zookeeper, only lock-760 exist.

I am sorry I can't upload my original test code because current project is not open-source project.
I'll leave pseudo code for my case. multiple instances was running concurrently.
{code:java}
  abstract class BaseWatcher implements CuratorWatcher {
    private InterProcessMutex lock;
    private final String objectPath = "/object";
    private final String lockPath = "/lock";
    private String namespace;

    BaseWatcher() {
      this.lock = new InterProcessMutex(this.zooKeeperClient, this.namespace + lockPath);
    }

    @Override
    public abstract void process(WatchedEvent watchedEvent);

    protected void lock() throws Exception {
      this.lock.acquire();
    }

    protected void unlock() throws Exception {
      if (this.lock.isAcquiredInThisProcess()) {
        this.lock.release();
      }
    }

    protected void registerWatcher(CuratorWatcher watcher, boolean isWatcher2) throws Exception {
      if (isWatcher2) {
        this.zooKeeperClient.checkExists().usingWatcher(watcher).forPath(namespace + PathforWatcher2);
      } else {
        this.zooKeeperClient.checkExists().usingWatcher(watcher).forPath(namespace + objectPath);
      }
    }

    protected void updateObject(Byte[] data){
      this.zooKeeperClient.setData().forPath(namespace + objectPath, data);
    }
  }

  class Watcher1 extends BaseWatcher {

    @Override
    public void process(WatchedEvent watchedEvent) {
      try {
        super.lock();
        registerWatcher(this, false);
        super.unlock();
      } 
    }
  }

  class Watcher2 extends BaseWatcher {

    @Override
    public void process(WatchedEvent watchedEvent) {
      try {
        super.lock();
        super.updateObject(contents);
        super.unlock();
        super.registerWatcher(this, true);
      }
    }
  }

{code}



was (Author: jc5201):
[~randgalt] The case I said was node deleted after getData() called and before wait() called. 
I checked from thread dump that localvariable for lock-759(previousSequencePath) and lock-760(ourPath) exist but in zookeeper, only lock-760 exist.

I am sorry I can't upload my original test code because current project is not open-source project.
I'll leave pseudo code for my case. multiple instances was running concurrently.
{code:java}
  private abstract class BaseWatcher implements CuratorWatcher {
    private InterProcessMutex lock;
    private final String objectPath = "/object";
    private final String lockPath = "/lock";
    private String namespace;

    BaseWatcher() {
      this.lock = new InterProcessMutex(this.zooKeeperClient, this.namespace + lockPath);
    }

    @Override
    public abstract void process(WatchedEvent watchedEvent);

    protected void lock() throws Exception {
      this.lock.acquire();
    }

    protected void unlock() throws Exception {
      if (this.lock.isAcquiredInThisProcess()) {
        this.lock.release();
      }
    }

    protected void registerWatcher(CuratorWatcher watcher, boolean isWatcher2) throws Exception {
      if (isWatcher2) {
        this.zooKeeperClient.checkExists().usingWatcher(watcher).forPath(namespace + PathforWatcher2);
      } else {
        this.zooKeeperClient.checkExists().usingWatcher(watcher).forPath(namespace + objectPath);
      }
    }

    protected void updateObject(Byte[] data){
      this.zooKeeperClient.setData().forPath(namespace + objectPath, data);
    }
  }

  private class Watcher1 extends BaseWatcher {

    @Override
    public void process(WatchedEvent watchedEvent) {
      try {
        super.lock();
        registerWatcher(this, false);
        super.unlock();
      } 
    }
  }

  private class Watcher2 extends BaseWatcher {

    @Override
    public void process(WatchedEvent watchedEvent) {
      try {
        super.lock();
        super.updateObject(contents);
        super.unlock();
        super.registerWatcher(this, true);
      }
    }
  }

{code}


> Concurrency issue in LockInternals
> ----------------------------------
>
>                 Key: CURATOR-527
>                 URL: https://issues.apache.org/jira/browse/CURATOR-527
>             Project: Apache Curator
>          Issue Type: Bug
>          Components: Recipes
>    Affects Versions: 2.12.0
>         Environment: Curator 2.12.0
> zookeeper 3.4.14
>            Reporter: Kim Jaechang
>            Priority: Major
>
> I'm using InterProcessMutex and InterProcessMutex often failed to acquire lock.
> In LockInternals.internalLockLoop(), watcher is registered to zookeeper and call wait() like below
> {code:java}
> client.getData().usingWatcher(watcher).forPath(previousSequencePath);
> if ( millisToWait != null )
> {
>     millisToWait -= (System.currentTimeMillis() - startMillis);
>     startMillis = System.currentTimeMillis();
>     if ( millisToWait <= 0 )
>     {
>         doDelete = true;    // timed out - delete our node
>         break;
>     }
>     wait(millisToWait);
> }
> else
> {
>     wait();
> }
> {code}
> In my case, my program is waiting previousSequencePath=_c_f290140d-9856-42ad-b9bf-348ffc086062-lock-0000000759 to be deleted.
> But _c_f290140d-9856-42ad-b9bf-348ffc086062-lock-0000000759 is deleted between client.getData() and wait().
> if _c_f290140d-9856-42ad-b9bf-348ffc086062-lock-0000000759 is deleted when 
> client.getData().usingWatcher(watcher).forPath(previousSequencePath) is called, it will throw Exception but it was exist at that time.
> I'm using Curator 2.12.0 but latest version seems to have same issue.



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