[ 
https://issues.apache.org/jira/browse/CURATOR-626?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Viktor Feklin updated CURATOR-626:
----------------------------------
    Description: 
NPE processing lock released event:
{code:java}
java.lang.NullPointerException: null
    at 
java.util.concurrent.CompletableFuture.screenExecutor(CompletableFuture.java:415)
    at 
java.util.concurrent.CompletableFuture.runAsync(CompletableFuture.java:1871)
    at 
org.apache.curator.framework.imps.CuratorFrameworkImpl.runSafe(CuratorFrameworkImpl.java:191)
    at 
org.apache.curator.framework.CuratorFramework.postSafeNotify(CuratorFramework.java:344)
    at 
org.apache.curator.framework.recipes.locks.LockInternals$2.process(LockInternals.java:69)
    at 
org.apache.curator.framework.imps.NamespaceWatcher.process(NamespaceWatcher.java:77)
    at 
org.apache.zookeeper.ClientCnxn$EventThread.processEvent(ClientCnxn.java:535)
    at org.apache.zookeeper.ClientCnxn$EventThread.run(ClientCnxn.java:510) 
{code}
Steps to reproduce:
{code:java}
InterProcessMutex lock = new 
InterProcessMutex(curatorFramework.usingNamespace(null), path);
lock.acquire();{code}
If lock is holded by another process (at the moment of call) - we will never 
acquire it (after lock was released by holder) because notification event lost 
while processing on watcher.

The root cause of bug - is wrong init code of СuratorFrameworkImpl:
{code:java}
public CuratorFrameworkImpl(CuratorFrameworkFactory.Builder builder)
{
    .... 
    failedDeleteManager = new FailedDeleteManager(this);
    failedRemoveWatcherManager = new FailedRemoveWatchManager(this);
    
    // here you pass not fully initialized instance (this)
    namespaceFacadeCache = new NamespaceFacadeCache(this);

    ensembleTracker = zk34CompatibilityMode ? null : new EnsembleTracker(this, 
builder.getEnsembleProvider());

    runSafeService = makeRunSafeService(builder);
} {code}
In NamespaceFacadeCache:
{code:java}
NamespaceFacadeCache(CuratorFrameworkImpl client)
{
    this.client = client;
    // here you create facade for null namespace based on not fully initialized 
client 
    nullNamespace = new NamespaceFacade(client, null);
} {code}
NamespaceFacade - clones client fields, but not all fields initialized at this 
moment (ensembleTracker  and runSafeService - are both nulls).

So then we use null namespace - we use this broken client and get NPE on access 
this null fields (se stacktrace).

Fix is very easy:
{code:java}
public CuratorFrameworkImpl(CuratorFrameworkFactory.Builder builder)
{
    ...
    failedDeleteManager = new FailedDeleteManager(this);
    failedRemoveWatcherManager = new FailedRemoveWatchManager(this);

    ensembleTracker = zk34CompatibilityMode ? null : new EnsembleTracker(this, 
builder.getEnsembleProvider());

    runSafeService = makeRunSafeService(builder);
    
    // initialization of cache should be the last operation in init method (all 
fields are initialized)
    namespaceFacadeCache = new NamespaceFacadeCache(this);
}  {code}
 

  was:
NPE processing lock released event:
{code:java}
java.lang.NullPointerException: null
    at 
java.util.concurrent.CompletableFuture.screenExecutor(CompletableFuture.java:415)
    at 
java.util.concurrent.CompletableFuture.runAsync(CompletableFuture.java:1871)
    at 
org.apache.curator.framework.imps.CuratorFrameworkImpl.runSafe(CuratorFrameworkImpl.java:191)
    at 
org.apache.curator.framework.CuratorFramework.postSafeNotify(CuratorFramework.java:344)
    at 
org.apache.curator.framework.recipes.locks.LockInternals$2.process(LockInternals.java:69)
    at 
org.apache.curator.framework.imps.NamespaceWatcher.process(NamespaceWatcher.java:77)
    at 
org.apache.zookeeper.ClientCnxn$EventThread.processEvent(ClientCnxn.java:535)
    at org.apache.zookeeper.ClientCnxn$EventThread.run(ClientCnxn.java:510) 
{code}
Steps to reproduce:
{code:java}
InterProcessMutex lock = new 
InterProcessMutex(curatorFramework.usingNamespace(null), path);
lock.acquire();{code}
If lock is holded by another process (at the moment of call) - we will never 
acquire it (after lock was released by holder) because notification event lost 
while processing on watcher.

The root cause of bug - is wrong init code of СuratorFrameworkImpl:
{code:java}
public CuratorFrameworkImpl(CuratorFrameworkFactory.Builder builder)
{
    .... 
    failedDeleteManager = new FailedDeleteManager(this);
    failedRemoveWatcherManager = new FailedRemoveWatchManager(this);
    
    // here you pass not fully initialized instance (this)
    namespaceFacadeCache = new NamespaceFacadeCache(this);

    ensembleTracker = zk34CompatibilityMode ? null : new EnsembleTracker(this, 
builder.getEnsembleProvider());

    runSafeService = makeRunSafeService(builder);
} {code}
In NamespaceFacadeCache:
{code:java}
NamespaceFacadeCache(CuratorFrameworkImpl client)
{
    this.client = client;
    // here you create facade for null namespace based on not fully initialized 
client 
    nullNamespace = new NamespaceFacade(client, null);
} {code}
NamespaceFacade - clones client fields, but not all fields initialized at this 
moment:

ensembleTracker  and runSafeService - are both nulls

So then we use null namespace - we use this broken client and get NPE on access 
this null fields (se stacktrace).

Fix is very easy:
{code:java}
public CuratorFrameworkImpl(CuratorFrameworkFactory.Builder builder)
{
    ...
    failedDeleteManager = new FailedDeleteManager(this);
    failedRemoveWatcherManager = new FailedRemoveWatchManager(this);

    ensembleTracker = zk34CompatibilityMode ? null : new EnsembleTracker(this, 
builder.getEnsembleProvider());

    runSafeService = makeRunSafeService(builder);
    
    // initialization of cache should be the last operation in init method (all 
fields are initialized)
    namespaceFacadeCache = new NamespaceFacadeCache(this);
}  {code}
 


> NullPointerException in watcher of nullNamespace
> ------------------------------------------------
>
>                 Key: CURATOR-626
>                 URL: https://issues.apache.org/jira/browse/CURATOR-626
>             Project: Apache Curator
>          Issue Type: Bug
>          Components: Framework
>    Affects Versions: 5.2.0
>            Reporter: Viktor Feklin
>            Priority: Major
>
> NPE processing lock released event:
> {code:java}
> java.lang.NullPointerException: null
>     at 
> java.util.concurrent.CompletableFuture.screenExecutor(CompletableFuture.java:415)
>     at 
> java.util.concurrent.CompletableFuture.runAsync(CompletableFuture.java:1871)
>     at 
> org.apache.curator.framework.imps.CuratorFrameworkImpl.runSafe(CuratorFrameworkImpl.java:191)
>     at 
> org.apache.curator.framework.CuratorFramework.postSafeNotify(CuratorFramework.java:344)
>     at 
> org.apache.curator.framework.recipes.locks.LockInternals$2.process(LockInternals.java:69)
>     at 
> org.apache.curator.framework.imps.NamespaceWatcher.process(NamespaceWatcher.java:77)
>     at 
> org.apache.zookeeper.ClientCnxn$EventThread.processEvent(ClientCnxn.java:535)
>     at org.apache.zookeeper.ClientCnxn$EventThread.run(ClientCnxn.java:510) 
> {code}
> Steps to reproduce:
> {code:java}
> InterProcessMutex lock = new 
> InterProcessMutex(curatorFramework.usingNamespace(null), path);
> lock.acquire();{code}
> If lock is holded by another process (at the moment of call) - we will never 
> acquire it (after lock was released by holder) because notification event 
> lost while processing on watcher.
> The root cause of bug - is wrong init code of СuratorFrameworkImpl:
> {code:java}
> public CuratorFrameworkImpl(CuratorFrameworkFactory.Builder builder)
> {
>     .... 
>     failedDeleteManager = new FailedDeleteManager(this);
>     failedRemoveWatcherManager = new FailedRemoveWatchManager(this);
>     
>     // here you pass not fully initialized instance (this)
>     namespaceFacadeCache = new NamespaceFacadeCache(this);
>     ensembleTracker = zk34CompatibilityMode ? null : new 
> EnsembleTracker(this, builder.getEnsembleProvider());
>     runSafeService = makeRunSafeService(builder);
> } {code}
> In NamespaceFacadeCache:
> {code:java}
> NamespaceFacadeCache(CuratorFrameworkImpl client)
> {
>     this.client = client;
>     // here you create facade for null namespace based on not fully 
> initialized client 
>     nullNamespace = new NamespaceFacade(client, null);
> } {code}
> NamespaceFacade - clones client fields, but not all fields initialized at 
> this moment (ensembleTracker  and runSafeService - are both nulls).
> So then we use null namespace - we use this broken client and get NPE on 
> access this null fields (se stacktrace).
> Fix is very easy:
> {code:java}
> public CuratorFrameworkImpl(CuratorFrameworkFactory.Builder builder)
> {
>     ...
>     failedDeleteManager = new FailedDeleteManager(this);
>     failedRemoveWatcherManager = new FailedRemoveWatchManager(this);
>     ensembleTracker = zk34CompatibilityMode ? null : new 
> EnsembleTracker(this, builder.getEnsembleProvider());
>     runSafeService = makeRunSafeService(builder);
>     
>     // initialization of cache should be the last operation in init method 
> (all fields are initialized)
>     namespaceFacadeCache = new NamespaceFacadeCache(this);
> }  {code}
>  



--
This message was sent by Atlassian Jira
(v8.20.1#820001)

Reply via email to