Author: ron.sigal(a)jboss.com Date: 2014-12-10 23:39:06 -0500 (Wed, 10 Dec 2014) New Revision: 6530 Added: remoting2/branches/2.x/src/tests/org/jboss/test/remoting/connection/identity/ConnectionValidatorReuseTestCase.java Modified: remoting2/branches/2.x/src/main/org/jboss/remoting/ConnectionValidator.java remoting2/branches/2.x/src/main/org/jboss/remoting/Lease.java remoting2/branches/2.x/src/main/org/jboss/remoting/LeasePinger.java remoting2/branches/2.x/src/main/org/jboss/remoting/MicroRemoteClientInvoker.java remoting2/branches/2.x/src/main/org/jboss/remoting/ServerInvoker.java Log: JBREM-1328/JBREM-1329: 1) ConnectionValidator gets clientLeasePeriod at construction instead of a Client; 2) Lease$LeaseTimerTask synchronizes to avoid a race condition; 3) LeasePinger no longer uses a static Timer; 4) new ConnectionValidatorReuseTestCase; additional logging. Modified: remoting2/branches/2.x/src/main/org/jboss/remoting/ConnectionValidator.java =================================================================== --- remoting2/branches/2.x/src/main/org/jboss/remoting/ConnectionValidator.java 2014-12-09 06:18:54 UTC (rev 6529) +++ remoting2/branches/2.x/src/main/org/jboss/remoting/ConnectionValidator.java 2014-12-11 04:39:06 UTC (rev 6530) @@ -260,6 +260,7 @@ private volatile boolean stopping; private String invokerSessionId; private boolean tieToLease = true; + private long clientLeasePeriod; private boolean stopLeaseOnFailure = true; private int pingTimeout; private int failureDisconnectTimeout = -1; @@ -287,6 +288,7 @@ listeners = new HashMap(); stopped = false; getParameters(client, new HashMap()); + clientLeasePeriod = client.getLeasePeriod(); log.debug(this + " created"); } @@ -300,6 +302,7 @@ stopped = false; this.metadata = new HashMap(metadata); getParameters(client, metadata); + clientLeasePeriod = client.getLeasePeriod(); log.debug(this + " created"); } @@ -357,7 +360,9 @@ { isValid = false; - if (tieToLease && client.getLeasePeriod() > 0) + log.trace(this + ": tieToLease: " + tieToLease + ", client lease period: " + client.getLeasePeriod()); +// if (tieToLease && client.getLeasePeriod() > 0) + if (tieToLease && clientLeasePeriod > 0) { if (trace) { @@ -464,7 +469,9 @@ if (s.size() == 0) { listeners.remove(listener); + log.debug(this + " removed " + listener); } + log.debug(this + " listeners.size(): " + listeners.size()); if (listeners.size() == 0) { stop(); @@ -485,7 +492,7 @@ public String toString() { - return "ConnectionValidator[" + Integer.toHexString(System.identityHashCode(this)) + ":" + clientInvoker + ", pingPeriod=" + pingPeriod + " ms]"; + return "ConnectionValidator[" + Integer.toHexString(System.identityHashCode(this)) + ":" + client + ", " + clientInvoker + ", pingPeriod=" + pingPeriod + " ms]"; } public boolean isStopped() @@ -670,6 +677,7 @@ try { tieToLease = Boolean.valueOf(((String) o)).booleanValue(); + log.debug(this + " tieToLease: " + tieToLease); } catch (Exception e) { @@ -902,16 +910,16 @@ if (pingWorked) { - if (trace) { log.trace("ConnectionValidator got successful ping using " + clientInvoker);} + if (trace) { log.trace(this + " got successful ping using " + clientInvoker);} } else { - if (trace) { log.trace("ConnectionValidator did not get successful ping response " + clientInvoker);} + if (trace) { log.trace(this + " did not get successful ping response " + clientInvoker);} } } catch (Throwable t) { - log.debug("ConnectionValidator failed to ping via " + clientInvoker, t); + log.debug(this + " failed to ping via " + clientInvoker, t); } return pingWorked; @@ -982,16 +990,16 @@ if (pingWorked) { - if (trace) { log.trace("ConnectionValidator got successful ping using " + clientInvoker);} + if (trace) { log.trace(this + " got successful ping using " + clientInvoker);} } else { - if (trace) { log.trace("ConnectionValidator did not get successful ping response " + clientInvoker);} + if (trace) { log.trace(this + " did not get successful ping response " + clientInvoker);} } } catch (Throwable t) { - log.debug("ConnectionValidator failed to ping via " + clientInvoker, t); + log.debug(this + " failed to ping via " + clientInvoker, t); } return pingWorked; Modified: remoting2/branches/2.x/src/main/org/jboss/remoting/Lease.java =================================================================== --- remoting2/branches/2.x/src/main/org/jboss/remoting/Lease.java 2014-12-09 06:18:54 UTC (rev 6529) +++ remoting2/branches/2.x/src/main/org/jboss/remoting/Lease.java 2014-12-11 04:39:06 UTC (rev 6530) @@ -87,6 +87,7 @@ } this.leaseWindow = leasePeriod * 2; this.clientLeases = clientLeases; + log.debug("clientLeases: " + clientLeases); } @@ -226,7 +227,7 @@ public String toString() { String hash = Integer.toHexString(System.identityHashCode(this)); - return "Lease[" + hash + ":" + clientSessionId + ":" + leasePingerId + "]"; + return "Lease[" + hash + ":" + clientSessionId + ":" + leasePingerId + ":" + leaseId + "]"; } private void notifyClientTermination(String sessionId) @@ -400,22 +401,51 @@ */ public void run() { - if (leaseUpdated) + boolean timedOut = false; + + if (clientLeases != null) { - leaseUpdated = false; + synchronized (clientLeases) + { + // ServerInvoker might be looking for a Lease. + if (leaseUpdated) + { + leaseUpdated = false; + } + else + { + timedOut = true; + if (clientLeases.remove(clientSessionId) != null) + { + if (isTraceEnabled) log.trace(Lease.this + " removed lease:" + clientSessionId); + } + else + { + if (isTraceEnabled) log.trace(Lease.this + " clientSessionId not found in clientLeases"); + } + } + } } else { + if (leaseUpdated) + { + leaseUpdated = false; + } + else + { + timedOut = true; + } + } + + if (timedOut) + { + if (isTraceEnabled) log.trace(Lease.this + " did not receive ping: " + clientSessionId); + stopLease(); + try { - if (isTraceEnabled) log.trace(Lease.this + " did not receive ping: " + clientSessionId); - stopLease(); notifyClientLost(); - if (clientLeases != null) - { - clientLeases.remove(clientSessionId); - } - if (isTraceEnabled) log.trace(Lease.this + " removed lease:" + clientSessionId); } catch (Throwable thr) { Modified: remoting2/branches/2.x/src/main/org/jboss/remoting/LeasePinger.java =================================================================== --- remoting2/branches/2.x/src/main/org/jboss/remoting/LeasePinger.java 2014-12-09 06:18:54 UTC (rev 6529) +++ remoting2/branches/2.x/src/main/org/jboss/remoting/LeasePinger.java 2014-12-11 04:39:06 UTC (rev 6530) @@ -33,7 +33,7 @@ private static boolean trace = log.isTraceEnabled(); - private static Timer timer = new Timer(true); + private Timer timer = new Timer(true); // Attributes ----------------------------------------------------------------------------------- @@ -321,6 +321,7 @@ public long getLeasePeriod(String sessionID) { + log.trace(this + ": timerTask: " + timerTask + ", clientSessionIds.containsKey(sessionID): " + clientSessionIds.containsKey(sessionID)); if (timerTask == null) { return -1; Modified: remoting2/branches/2.x/src/main/org/jboss/remoting/MicroRemoteClientInvoker.java =================================================================== --- remoting2/branches/2.x/src/main/org/jboss/remoting/MicroRemoteClientInvoker.java 2014-12-09 06:18:54 UTC (rev 6529) +++ remoting2/branches/2.x/src/main/org/jboss/remoting/MicroRemoteClientInvoker.java 2014-12-11 04:39:06 UTC (rev 6530) @@ -472,6 +472,7 @@ { synchronized(clientLeaseLock) { + log.trace(this + ": leasePinger: " + leasePinger); if(leasePinger == null) { return -1; Modified: remoting2/branches/2.x/src/main/org/jboss/remoting/ServerInvoker.java =================================================================== --- remoting2/branches/2.x/src/main/org/jboss/remoting/ServerInvoker.java 2014-12-09 06:18:54 UTC (rev 6529) +++ remoting2/branches/2.x/src/main/org/jboss/remoting/ServerInvoker.java 2014-12-11 04:39:06 UTC (rev 6530) @@ -449,6 +449,7 @@ public void removeConnectionListener(ConnectionListener listener) { + log.debug(this + " removing ConnecttionListener " + listener); if(connectionNotifier != null) { connectionNotifier.removeListener(listener); @@ -466,6 +467,7 @@ String sessionId = (String)itr.next(); Lease clientLease = (Lease)clientLeases.get(sessionId); clientLease.terminateLease(sessionId); + log.debug(this + " terminating " + clientLease); } clientLeases.clear(); } @@ -2064,9 +2066,25 @@ } if(clientSessionId != null) { - if(trace) { log.trace("Getting lease for invoker session id: " + clientSessionId); } - - Lease clientLease = (Lease)clientLeases.get(clientSessionId); + if(trace) + { + log.trace("Getting lease for invoker session id: " + clientSessionId); + log.trace(this + ": existing leases:"); + Iterator it = clientLeases.keySet().iterator(); + while (it.hasNext()) + { + Object key = it.next(); + log.trace(" " + clientLeases.get(key)); + } + } + + Lease clientLease = null; + synchronized (clientLeases) + { + // Lease$LeaseTimerTask might be removing a Lease that timed out. + clientLease = (Lease)clientLeases.get(clientSessionId); + } + if(clientLease == null) { Lease newClientLease = new Lease(clientSessionId, leasePeriod, @@ -2099,7 +2117,7 @@ if (trace) log.trace("terminating invoker lease: " + clientLease); clientLease.terminateLeaseUponFailure(clientSessionId); clientLeases.remove(clientSessionId); - + if (trace) log.trace(this + " clientLeases: " + clientLeases); Lease newClientLease = new Lease(clientSessionId, leasePeriod, locator.getLocatorURI(), invocation.getRequestPayload(), Added: remoting2/branches/2.x/src/tests/org/jboss/test/remoting/connection/identity/ConnectionValidatorReuseTestCase.java =================================================================== --- remoting2/branches/2.x/src/tests/org/jboss/test/remoting/connection/identity/ConnectionValidatorReuseTestCase.java (rev 0) +++ remoting2/branches/2.x/src/tests/org/jboss/test/remoting/connection/identity/ConnectionValidatorReuseTestCase.java 2014-12-11 04:39:06 UTC (rev 6530) @@ -0,0 +1,263 @@ +/* +* JBoss, Home of Professional Open Source +* Copyright 2009, JBoss Inc., and individual contributors as indicated +* by the @authors tag. See the copyright.txt in the distribution for a +* full listing of individual contributors. +* +* This is free software; you can redistribute it and/or modify it +* under the terms of the GNU Lesser General Public License as +* published by the Free Software Foundation; either version 2.1 of +* the License, or (at your option) any later version. +* +* This software is distributed in the hope that it will be useful, +* but WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +* Lesser General Public License for more details. +* +* You should have received a copy of the GNU Lesser General Public +* License along with this software; if not, write to the Free +* Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA +* 02110-1301 USA, or see the FSF site: http://www.fsf.org. +*/ +package org.jboss.test.remoting.connection.identity; + +import java.net.InetAddress; +import java.util.HashMap; +import java.util.Map; + +import javax.management.MBeanServer; + +import junit.framework.Assert; +import junit.framework.TestCase; + +import org.apache.log4j.ConsoleAppender; +import org.apache.log4j.Level; +import org.apache.log4j.Logger; +import org.apache.log4j.PatternLayout; +import org.jboss.remoting.Client; +import org.jboss.remoting.ConnectionListener; +import org.jboss.remoting.ConnectionValidator; +import org.jboss.remoting.InvocationRequest; +import org.jboss.remoting.InvokerLocator; +import org.jboss.remoting.Remoting; +import org.jboss.remoting.ServerInvocationHandler; +import org.jboss.remoting.ServerInvoker; +import org.jboss.remoting.callback.InvokerCallbackHandler; +import org.jboss.remoting.transport.Connector; +import org.jboss.remoting.transport.PortUtil; +import org.jboss.remoting.transport.socket.SocketServerInvoker; + + +/** + * + * @author <a href="mailto:ron.sigal@jboss.com">Ron Sigal</a> + * @version $Rev$ + * <p> + * Copyright Nov 5, 2014 + * </p> + */ +public class ConnectionValidatorReuseTestCase extends TestCase +{ + protected static final int LEASE_PERIOD = 2000; + protected static final int VALIDATOR_PING_TIMEOUT = 1000; + protected static final int VALIDATOR_PING_PERIOD = 2000; + protected static final int PING_PERIODS_TO_WAIT = 2; + + private static Logger log = Logger.getLogger(ConnectionValidatorReuseTestCase.class); + private static boolean firstTime = true; + + protected String host; + protected int port; + protected String locatorURI; + protected InvokerLocator serverLocator; + protected Connector connector; + protected TestInvocationHandler invocationHandler; + + + public void setUp() throws Exception + { + if (firstTime) + { + firstTime = false; + Logger.getLogger("org.jboss.remoting").setLevel(Level.TRACE); + Logger.getLogger("org.jboss.test.remoting").setLevel(Level.INFO); + String pattern = "[%d{ABSOLUTE}] [%t] %5p (%F:%L) - %m%n"; + PatternLayout layout = new PatternLayout(pattern); + ConsoleAppender consoleAppender = new ConsoleAppender(layout); + Logger.getRootLogger().addAppender(consoleAppender); + } + host = InetAddress.getLocalHost().getHostAddress(); + port = PortUtil.findFreePort(host); + } + + + public void tearDown() + { + } + + + public void testSimultaneousClients() throws Throwable + { + log.info("entering " + getName()); + + // Start server. + setupServer(); + + // Create first client. + InvokerLocator clientLocator = new InvokerLocator(locatorURI); + HashMap clientConfig = new HashMap(clientLocator.getParameters()); + clientConfig.put(InvokerLocator.FORCE_REMOTE, "true"); + clientConfig.put(Client.ENABLE_LEASE, "true"); + clientConfig.put(ConnectionValidator.VALIDATOR_PING_PERIOD, Integer.toString(VALIDATOR_PING_PERIOD)); + clientConfig.put(ConnectionValidator.VALIDATOR_PING_TIMEOUT, Integer.toString(VALIDATOR_PING_TIMEOUT)); + clientConfig.put(ConnectionValidator.FAILURE_DISCONNECT_TIMEOUT, "0"); + addExtraClientConfig(clientConfig); + Client client1 = new Client(clientLocator, clientConfig); + TestConnectionListener clientConnectionListener1 = new TestConnectionListener(); + client1.connect(clientConnectionListener1, null); + log.info("client1 is connected"); + + // Create second client. + Client client2 = new Client(clientLocator, clientConfig); + TestConnectionListener clientConnectionListener2 = new TestConnectionListener(); + client2.connect(clientConnectionListener2, null); + log.info("client2 is connected"); + + // Test connection. + assertEquals("abc", client1.invoke("abc")); + log.info("connection 1 is good"); + assertEquals("abc", client2.invoke("abc")); + log.info("connection 2 is good"); + + // Verify that ConnectionValidator is still tied to Lease. + Thread.sleep(4000); + client1.removeConnectionListener(clientConnectionListener1); + client1.disconnect(); + Thread.sleep(4000); + log.info("clientConnectionListener2.connectionFailed: " + clientConnectionListener2.connectionFailed); + Assert.assertFalse(clientConnectionListener2.connectionFailed); + + client2.removeConnectionListener(clientConnectionListener1); + client2.disconnect(); + shutdownServer(); + log.info(getName() + " PASSES"); + } + + + public void testSerialClients() throws Throwable + { + log.info("entering " + getName()); + + // Start server. + setupServer(); + + // Create first client. + InvokerLocator clientLocator = new InvokerLocator(locatorURI); + HashMap clientConfig = new HashMap(clientLocator.getParameters()); + clientConfig.put(InvokerLocator.FORCE_REMOTE, "true"); + clientConfig.put(Client.ENABLE_LEASE, "true"); + clientConfig.put(ConnectionValidator.VALIDATOR_PING_PERIOD, Integer.toString(VALIDATOR_PING_PERIOD)); + clientConfig.put(ConnectionValidator.VALIDATOR_PING_TIMEOUT, Integer.toString(VALIDATOR_PING_TIMEOUT)); + clientConfig.put(ConnectionValidator.FAILURE_DISCONNECT_TIMEOUT, "0"); + addExtraClientConfig(clientConfig); + Client client1 = new Client(clientLocator, clientConfig); + TestConnectionListener clientConnectionListener1 = new TestConnectionListener(); + client1.connect(clientConnectionListener1, null); + log.info("client1 is connected"); + assertEquals("abc", client1.invoke("abc")); + log.info("connection 1 is good"); + + // Create second client. + Client client2 = new Client(clientLocator, clientConfig); + TestConnectionListener clientConnectionListener2 = new TestConnectionListener(); + client2.connect(clientConnectionListener2, null); + log.info("client2 is connected"); + assertEquals("abc", client2.invoke("abc")); + log.info("connection 2 is good"); + + // Disconnect first client. + Thread.sleep(4000); + client1.removeConnectionListener(clientConnectionListener1); + client1.disconnect(); + Thread.sleep(4000); + + // Verify that ConnectionValidator gets updated with client2 + log.info("clientConnectionListener2.connectionFailed: " + clientConnectionListener2.connectionFailed); + Assert.assertFalse(clientConnectionListener2.connectionFailed); + + client2.removeConnectionListener(clientConnectionListener1); + client2.disconnect(); + shutdownServer(); + log.info(getName() + " PASSES"); + } + + + protected String getTransport() + { + return "socket"; + } + + + protected void addExtraClientConfig(Map config) {} + protected void addExtraServerConfig(Map config) {} + + + protected void setupServer() throws Exception + { + locatorURI = getTransport() + "://" + host + ":" + port + "?" + Remoting.USE_SERVER_CONNECTION_IDENTITY + "=true"; + locatorURI += "&" + Remoting.USE_CLIENT_CONNECTION_IDENTITY + "=true"; + locatorURI += "&" + SocketServerInvoker.CHECK_CONNECTION_KEY + "=true"; + locatorURI += "&" + InvokerLocator.CLIENT_LEASE + "=true"; + locatorURI += "&" + InvokerLocator.CLIENT_LEASE_PERIOD + "=" + LEASE_PERIOD; + String metadata = System.getProperty("remoting.metadata"); + if (metadata != null) + { + locatorURI += "&" + metadata; + } + serverLocator = new InvokerLocator(locatorURI); + log.info("Starting remoting server with locator uri of: " + locatorURI); + HashMap config = new HashMap(); + config.put(InvokerLocator.FORCE_REMOTE, "true"); + addExtraServerConfig(config); + connector = new Connector(serverLocator, config); + connector.create(); + invocationHandler = new TestInvocationHandler(); + connector.addInvocationHandler("test", invocationHandler); + connector.addConnectionListener(new TestConnectionListener()); + connector.start(); + } + + + protected void shutdownServer() throws Exception + { + if (connector != null) + connector.stop(); + } + + + static class TestInvocationHandler implements ServerInvocationHandler + { + public void addListener(InvokerCallbackHandler callbackHandler) {} + public Object invoke(final InvocationRequest invocation) throws Throwable + { + return invocation.getParameter(); + } + public void removeListener(InvokerCallbackHandler callbackHandler) {} + public void setMBeanServer(MBeanServer server) {} + public void setInvoker(ServerInvoker invoker) {} + } + + + static class TestConnectionListener implements ConnectionListener + { + public boolean connectionFailed; + public Throwable throwable; + + public void handleConnectionException(Throwable throwable, Client client) + { + connectionFailed = true; + this.throwable = throwable; + log.info(this + " received connection notification: connectionFailed: " + connectionFailed); + } + } +} \ No newline at end of file Property changes on: remoting2/branches/2.x/src/tests/org/jboss/test/remoting/connection/identity/ConnectionValidatorReuseTestCase.java ___________________________________________________________________ Added: svn:mime-type + text/plain