[CURATOR-626] NullPointerException in watcher of nullNamespace

• Type: Bug
• Status: Resolved
• Priority: Major
• Resolution: Fixed
• Affects Version/s: 5.2.0
• Fix Version/s: 5.6.0
• Component/s: Framework
• Assignee: Kezhu Wang
• Reporter: Viktor Feklin
• Created: 30/Dec/21 05:34
• Updated: 13/Aug/23 04:24
• Resolved: 13/Aug/23 04:24

Description

NPE processing lock released event:

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) 

Steps to reproduce:

InterProcessMutex lock = new InterProcessMutex(curatorFramework.usingNamespace(null), path);
lock.acquire();

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:

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);
} 

In NamespaceFacadeCache:

NamespaceFacadeCache(CuratorFrameworkImpl client)
{
    this.client = client;
    // here you create facade for null namespace based on not fully initialized client 
    nullNamespace = new NamespaceFacade(client, null);
} 

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:

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);
}  

 

Issue Links

• links to: GitHub Pull Request #475