[curator] branch master updated: CURATOR-575: TestingServer shutdown can cause an NPE

[eolivelli]

This is an automated email from the ASF dual-hosted git repository.

eolivelli pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/curator.git


The following commit(s) were added to refs/heads/master by this push:
     new 3e2d52e  CURATOR-575: TestingServer shutdown can cause an NPE
3e2d52e is described below

commit 3e2d52e2998cc32697f369a18492fdb8b88baf90
Author: randgalt <randg...@apache.org>
AuthorDate: Thu Jun 25 08:46:08 2020 +0200

    CURATOR-575: TestingServer shutdown can cause an NPE
    
    TestingServer shutdown can cause an NPE due to FileTxnSnapLog being closed 
in a different thread
    
    Author: randgalt <randg...@apache.org>
    
    Reviewers: Enrico Olivelli <eolive...@apache.org>, Cameron McKenzie 
<mckenzie....@gmail.com>
    
    Closes #368 from Randgalt/CURATOR-575-fix-testing-server-npe
---
 .../apache/curator/test/TestingZooKeeperMain.java  | 24 ++++++++++++++++------
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git 
a/curator-test/src/main/java/org/apache/curator/test/TestingZooKeeperMain.java 
b/curator-test/src/main/java/org/apache/curator/test/TestingZooKeeperMain.java
index d53fb08..cdf59d0 100644
--- 
a/curator-test/src/main/java/org/apache/curator/test/TestingZooKeeperMain.java
+++ 
b/curator-test/src/main/java/org/apache/curator/test/TestingZooKeeperMain.java
@@ -226,13 +226,12 @@ public class TestingZooKeeperMain implements 
ZooKeeperMainFace
     private void internalRunFromConfig(ServerConfig config) throws IOException
     {
         log.info("Starting server");
-        FileTxnSnapLog txnLog = null;
         try {
             // Note that this thread isn't going to be doing anything else,
             // so rather than spawning another thread, we will just call
             // run() in this thread.
             // create a file logger url from the command line args
-            txnLog = new FileTxnSnapLog(config.getDataLogDir(), 
config.getDataDir());
+            FileTxnSnapLog txnLog = new FileTxnSnapLog(config.getDataLogDir(), 
config.getDataDir());
             zkServer = new TestZooKeeperServer(txnLog, config);
 
             try
@@ -261,22 +260,35 @@ public class TestingZooKeeperMain implements 
ZooKeeperMainFace
             // warn, but generally this is ok
             Thread.currentThread().interrupt();
             log.warn("Server interrupted", e);
-        } finally {
-            if (txnLog != null) {
-                txnLog.close();
-            }
         }
     }
 
     public static class TestZooKeeperServer extends ZooKeeperServer
     {
+        private final FileTxnSnapLog txnLog;
+
         public TestZooKeeperServer(FileTxnSnapLog txnLog, ServerConfig config)
         {
+            this.txnLog = txnLog;
             this.setTxnLogFactory(txnLog);
             this.setMinSessionTimeout(config.getMinSessionTimeout());
             this.setMaxSessionTimeout(config.getMaxSessionTimeout());
         }
 
+        @Override
+        public synchronized void shutdown(boolean fullyShutDown)
+        {
+            super.shutdown(fullyShutDown);
+            try
+            {
+                txnLog.close();
+            }
+            catch ( IOException e )
+            {
+                // ignore
+            }
+        }
+
         private final AtomicBoolean isRunning = new AtomicBoolean(false);
 
         public RequestProcessor getFirstProcessor()