[GEODE-2324] Make AcceptorImplObserver an interface and fix threading issues.
Project: http://git-wip-us.apache.org/repos/asf/geode/repo Commit: http://git-wip-us.apache.org/repos/asf/geode/commit/6bed2828 Tree: http://git-wip-us.apache.org/repos/asf/geode/tree/6bed2828 Diff: http://git-wip-us.apache.org/repos/asf/geode/diff/6bed2828 Branch: refs/heads/develop Commit: 6bed28285d8ff150c1759a2b04cb9959730aae51 Parents: 1d561dc Author: Galen O'Sullivan <gosulli...@pivotal.io> Authored: Tue Jan 24 15:28:25 2017 -0800 Committer: Bruce Schuchardt <bschucha...@pivotal.io> Committed: Thu Feb 9 14:28:48 2017 -0800 ---------------------------------------------------------------------- .../cache/tier/sockets/AcceptorImpl.java | 41 +++++++++++++++----- .../sockets/command/AcceptorImplObserver.java | 26 ++----------- .../tier/sockets/AcceptorImplDUnitTest.java | 13 ++++++- 3 files changed, 46 insertions(+), 34 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/geode/blob/6bed2828/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/AcceptorImpl.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/AcceptorImpl.java b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/AcceptorImpl.java index d72c72b..1bca1fd 100755 --- a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/AcceptorImpl.java +++ b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/AcceptorImpl.java @@ -271,6 +271,33 @@ public class AcceptorImpl extends Acceptor implements Runnable { private SecurityService securityService = IntegratedSecurityService.getSecurityService(); + // Assumed non-null. Do not set this to null. + private static AcceptorImplObserver acceptorImplObserver_do_not_access_directly = + new AcceptorImplObserver() { + @Override + public void beforeClose(AcceptorImpl acceptorImpl) {} + + @Override + public void normalCloseTermination(AcceptorImpl acceptorImpl) {} + + @Override + public void afterClose(AcceptorImpl acceptorImpl) {} + }; + + private static AcceptorImplObserver getAcceptorImplObserver() { + synchronized (AcceptorImpl.class) { + return acceptorImplObserver_do_not_access_directly; + } + } + + public static void setObserver_TESTONLY(AcceptorImplObserver observer) { + synchronized (AcceptorImpl.class) { + if (observer != null) { + acceptorImplObserver_do_not_access_directly = observer; + } + } + } + /** * Initializes this acceptor thread to listen for connections on the given port. * @@ -1529,12 +1556,10 @@ public class AcceptorImpl extends Acceptor implements Runnable { @Override public void close() { - AcceptorImplObserver acceptorImplObserver = AcceptorImplObserver.getInstance(); + AcceptorImplObserver acceptorImplObserver = getAcceptorImplObserver(); try { synchronized (syncLock) { - if (acceptorImplObserver != null) { - acceptorImplObserver.beforeClose(this); - } + acceptorImplObserver.beforeClose(this); if (!isRunning()) { return; } @@ -1613,16 +1638,12 @@ public class AcceptorImpl extends Acceptor implements Runnable { } } } - if (acceptorImplObserver != null) { - acceptorImplObserver.normalCloseTermination(this); - } + acceptorImplObserver.normalCloseTermination(this); } // synchronized } catch (RuntimeException e) {/* ignore and log */ logger.warn(LocalizedMessage.create(LocalizedStrings.AcceptorImpl_UNEXPECTED), e); } finally { - if (acceptorImplObserver != null) { - acceptorImplObserver.afterClose(this); - } + acceptorImplObserver.afterClose(this); } } http://git-wip-us.apache.org/repos/asf/geode/blob/6bed2828/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/AcceptorImplObserver.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/AcceptorImplObserver.java b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/AcceptorImplObserver.java index f5ee982..3d02878 100644 --- a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/AcceptorImplObserver.java +++ b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/AcceptorImplObserver.java @@ -20,28 +20,10 @@ import org.apache.geode.internal.cache.tier.sockets.AcceptorImpl; /** * AcceptorImplObserver is an observer/visitor for AcceptorImpl that is used for testing. */ -public abstract class AcceptorImplObserver { - private static AcceptorImplObserver instance; +public interface AcceptorImplObserver { + void beforeClose(AcceptorImpl acceptorImpl); - /** - * Set the instance of the observer. Setting to null will clear the observer. - * - * @param instance - * @return the old observer, or null if there was no old observer. - */ - public static final AcceptorImplObserver setInstance(AcceptorImplObserver instance) { - AcceptorImplObserver oldInstance = AcceptorImplObserver.instance; - AcceptorImplObserver.instance = instance; - return oldInstance; - } + void normalCloseTermination(AcceptorImpl acceptorImpl); - public static final AcceptorImplObserver getInstance() { - return instance; - } - - public void beforeClose(AcceptorImpl acceptorImpl) {} - - public void normalCloseTermination(AcceptorImpl acceptorImpl) {} - - public void afterClose(AcceptorImpl acceptorImpl) {} + void afterClose(AcceptorImpl acceptorImpl); } http://git-wip-us.apache.org/repos/asf/geode/blob/6bed2828/geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/AcceptorImplDUnitTest.java ---------------------------------------------------------------------- diff --git a/geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/AcceptorImplDUnitTest.java b/geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/AcceptorImplDUnitTest.java index 8b4c672..ca8592f 100644 --- a/geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/AcceptorImplDUnitTest.java +++ b/geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/AcceptorImplDUnitTest.java @@ -112,7 +112,7 @@ public class AcceptorImplDUnitTest extends JUnit4DistributedTestCase { Properties props = new Properties(); props.setProperty(MCAST_PORT, "0"); - AcceptorImplObserver.setInstance(new AcceptorImplObserver() { + AcceptorImpl.setObserver_TESTONLY(new AcceptorImplObserver() { @Override public void beforeClose(AcceptorImpl acceptorImpl) { Thread.currentThread().interrupt(); @@ -158,7 +158,16 @@ public class AcceptorImplDUnitTest extends JUnit4DistributedTestCase { assertTrue(passedPostConditions.get()); // cleanup. - AcceptorImplObserver.setInstance(null); + AcceptorImpl.setObserver_TESTONLY(new AcceptorImplObserver() { + @Override + public void beforeClose(AcceptorImpl acceptorImpl) {} + + @Override + public void normalCloseTermination(AcceptorImpl acceptorImpl) {} + + @Override + public void afterClose(AcceptorImpl acceptorImpl) {} + }); } } }