[jboss-cvs] JBossRemoting/src/main/org/jboss/remoting ...
Ovidiu Feodorov
ovidiu.feodorov at jboss.com
Wed Jan 10 05:30:33 EST 2007
User: ovidiu
Date: 07/01/10 05:30:33
Modified: src/main/org/jboss/remoting Tag: remoting_2_x
ConnectionValidator.java InvokerRegistry.java
LeasePinger.java MicroRemoteClientInvoker.java
Log:
Proposed fix for http://jira.jboss.org/jira/browse/JBREM-662
Even if functionality changes will be refactored, I would kindly request
that the extra log statements to be left in place.
Revision Changes Path
No revision
No revision
1.13.2.3 +52 -10 JBossRemoting/src/main/org/jboss/remoting/ConnectionValidator.java
(In the diff below, changes in quantity of whitespace are not shown.)
Index: ConnectionValidator.java
===================================================================
RCS file: /cvsroot/jboss/JBossRemoting/src/main/org/jboss/remoting/ConnectionValidator.java,v
retrieving revision 1.13.2.2
retrieving revision 1.13.2.3
diff -u -b -r1.13.2.2 -r1.13.2.3
--- ConnectionValidator.java 12 Dec 2006 14:41:16 -0000 1.13.2.2
+++ ConnectionValidator.java 10 Jan 2007 10:30:33 -0000 1.13.2.3
@@ -35,6 +35,7 @@
/**
* @author <a href="mailto:tom.elrod at jboss.com">Tom Elrod</a>
+ * @author <a href="mailto:ovidiu at jboss.org">Ovidiu Feodorov</a>
*/
public class ConnectionValidator extends TimerTask
{
@@ -44,6 +45,7 @@
private long pingPeriod = DEFAULT_PING_PERIOD;
protected static final Logger log = Logger.getLogger(ConnectionValidator.class.getName());
+ private static boolean trace = log.isTraceEnabled();
private Object lock = new Object();
private boolean stopped = false;
@@ -55,19 +57,23 @@
public ConnectionValidator(Client client)
{
- this.client = client;
+ this(client, (int)DEFAULT_PING_PERIOD);
}
public ConnectionValidator(Client client, int pingPeriod)
{
this.client = client;
this.pingPeriod = pingPeriod;
+
+ log.debug(this + " created");
}
private void start()
{
TimerUtil.schedule(this, pingPeriod);
stopped = false;
+
+ log.debug(this + " started");
}
public void stop()
@@ -81,6 +87,8 @@
}
stopped = true;
}
+
+ log.debug(this + " stopped");
}
@@ -127,14 +135,21 @@
{
try
{
- boolean isValid = checkConnection(client.getInvoker().getLocator(), client.getConfiguration());
+ if (trace) { log.trace(this + " pinging ..."); }
+
+ boolean isValid =
+ checkConnection(client.getInvoker().getLocator(), client.getConfiguration());
+
if (!isValid)
{
- notifyListeners(new Exception("Could not connect to server."));
+ log.debug(this + "'s connections is invalid");
+
+ notifyListeners(new Exception("Could not connect to server!"));
}
}
catch (Throwable thr)
{
+ log.debug(this + " got throwable while pinging", thr);
notifyListeners(thr);
}
}
@@ -188,37 +203,64 @@
try
{
-
innerClientInvoker = InvokerRegistry.createClientInvoker(locator, configMap);
if (!innerClientInvoker.isConnected())
{
+ if (trace) { log.trace("inner client invoker not connected, connecting ..."); }
innerClientInvoker.connect();
}
+
+ if (trace)
+ {
+ log.trace("ConnectionValidator pinging " +
+ innerClientInvoker.getLocator().getProtocol() + "://" +
+ innerClientInvoker.getLocator().getHost() + ":" +
+ innerClientInvoker.getLocator().getPort());
+ }
+
/**
* Sending null client id as don't want to trigger lease on server side.
* This also means that client connection validator will NOT impact client
* lease, so can not depend on it to maintain client lease with the server.
*/
- Object o = innerClientInvoker.invoke(new InvocationRequest(null, Subsystem.SELF,
- "$PING$", null, null, null));
+ innerClientInvoker.
+ invoke(new InvocationRequest(null, Subsystem.SELF, "$PING$", null, null, null));
+
pingWorked = true;
}
catch (Throwable throwable)
{
- log.debug("ConnectionValidator could not successfully ping server (" + innerClientInvoker.getLocator());
+ log.debug("ConnectionValidator failed to ping server " +
+ innerClientInvoker.getLocator().getProtocol() + "://" +
+ innerClientInvoker.getLocator().getHost() + ":" +
+ innerClientInvoker.getLocator().getPort(), throwable);
}
finally
{
if (innerClientInvoker != null)
{
+ if (pingWorked)
+ {
InvokerRegistry.destroyClientInvoker(locator, configMap);
}
+ else
+ {
+ // need to get rid of the client invoker that keeps spinning
+ InvokerRegistry.reallyDestroyClientInvoker(locator, configMap);
+ }
+ }
}
return pingWorked;
-
}
+ public String toString()
+ {
+ InvokerLocator locator = client.getInvoker().getLocator();
+ return "ConnectionValidator[" + locator.getProtocol() + "://" + locator.getHost() + ":" +
+ locator.getPort() + ", pingPeriod=" + pingPeriod + " ms]";
+ }
+
}
\ No newline at end of file
1.34.4.1 +98 -51 JBossRemoting/src/main/org/jboss/remoting/InvokerRegistry.java
(In the diff below, changes in quantity of whitespace are not shown.)
Index: InvokerRegistry.java
===================================================================
RCS file: /cvsroot/jboss/JBossRemoting/src/main/org/jboss/remoting/InvokerRegistry.java,v
retrieving revision 1.34
retrieving revision 1.34.4.1
diff -u -b -r1.34 -r1.34.4.1
--- InvokerRegistry.java 20 Jul 2006 16:58:31 -0000 1.34
+++ InvokerRegistry.java 10 Jan 2007 10:30:33 -0000 1.34.4.1
@@ -48,13 +48,15 @@
*
* @author <a href="mailto:jhaynie at vocalocity.net">Jeff Haynie</a>
* @author <a href="mailto:telrod at e2technologies.net">Tom Elrod</a>
- * @version $Revision: 1.34 $
+ * @version $Revision: 1.34.4.1 $
*/
public class InvokerRegistry
{
private static final Logger log = Logger.getLogger(InvokerRegistry.class);
+ private static boolean isTraceEnabled = log.isTraceEnabled();
+
private static final Map clientLocators = new HashMap();
private static final Map serverLocators = new HashMap();
@@ -67,8 +69,6 @@
/**
* return an array of InvokerLocators that are local to this VM (server invokers)
- *
- * @return
*/
public static synchronized final InvokerLocator[] getRegisteredServerLocators()
{
@@ -80,7 +80,6 @@
* a compatible transport
*
* @param remote
- * @return
*/
public static synchronized InvokerLocator getSuitableServerLocatorForRemote(InvokerLocator remote)
{
@@ -99,8 +98,6 @@
/**
* return an array of String of the registered transports
- *
- * @return
*/
public static final String[] getRegisteredInvokerTransports()
{
@@ -114,8 +111,6 @@
/**
* return an array of ClientInvokers that are connected
- *
- * @return
*/
public static final ClientInvoker[] getClientInvokers()
{
@@ -147,8 +142,6 @@
/**
* return an array of ServerInvokers that are connected
- *
- * @return
*/
public static final ServerInvoker[] getServerInvokers()
{
@@ -198,7 +191,6 @@
* returns true if the client invoker is registered in the local JVM for a given locator
*
* @param locator
- * @return
*/
public static boolean isClientInvokerRegistered(InvokerLocator locator)
{
@@ -209,19 +201,27 @@
}
/**
- * called to destroy any cached RemoteClientInvoker copies inside the registry. this method
+ * Called to destroy any cached RemoteClientInvoker copies inside the registry. This method
* must be called when it is determined that a remote server (via the locator) is no
* longer available.
- *
- * @param locator
*/
public static void destroyClientInvoker(InvokerLocator locator, Map configuration)
{
synchronized(clientLock)
{
+ if (isTraceEnabled)
+ {
+ log.trace("destroying client invoker " + locator + ", config " + configuration);
+ }
+
ClientInvoker invoker = decrementClientInvokerCounter(locator, configuration);
+
if(invoker != null)
{
+ if (isTraceEnabled)
+ {
+ log.trace("disconnecting " + invoker);
+ }
invoker.disconnect();
invoker = null;
}
@@ -229,6 +229,43 @@
}
/**
+ * (Temporary) method that fixes http://jira.jboss.org/jira/browse/JBREM-662.
+ * Must be refactored (renamed, to start with).
+ */
+ public static void reallyDestroyClientInvoker(InvokerLocator locator, Map configuration)
+ {
+ synchronized(clientLock)
+ {
+ if (isTraceEnabled)
+ {
+ log.trace("really destroying client invoker " + locator + ", config " + configuration);
+ }
+
+ // loop until last reference; really dangerous if the invoker is not in registry, because
+ // it will loop forever. decrementClientInvokerCounter() semantics must change to clearly
+ // indicate when it was a invoker and when it wasn't an invoker in registry.
+
+ int loops = 0;
+ ClientInvoker invoker = null;
+ while((invoker = decrementClientInvokerCounter(locator, configuration)) == null)
+ {
+ // loop, dangerous, a precautionary hack
+ if (++loops == 50)
+ {
+ throw new IllegalStateException("got into a hopeless loop!");
+ }
+ }
+
+ if (isTraceEnabled)
+ {
+ log.trace("disconnecting " + invoker);
+ }
+ invoker.disconnect();
+ invoker = null;
+ }
+ }
+
+ /**
* create a ClientInvoker instance, using the specific InvokerLocator, which is just a client-side
* invoker to a remote server. Will use the default configuration values for the transport.
*
@@ -363,7 +400,6 @@
* Note, this will also increment the internal reference count for the invoker
* @param locator
* @param configuration
- * @return
*/
private static ClientInvoker getRegisteredClientInvoker(InvokerLocator locator, Map configuration)
{
@@ -480,7 +516,6 @@
* returns true if the server invoker is registered in the local JVM for a given locator/handler pair
*
* @param locator
- * @return
*/
public static boolean isServerInvokerRegistered(InvokerLocator locator)
{
@@ -547,11 +582,17 @@
private static ClientInvoker decrementClientInvokerCounter(InvokerLocator locator, Map configuration)
{
- ClientInvoker clientInvoker = null;
- ClientInvokerHolder holder = null;
- List holderList = (List) clientLocators.get(locator);
- if (holderList != null)
+ List holderList = (List)clientLocators.get(locator);
+
+ if (holderList == null)
{
+ log.debug("Could not decrement client invoker counter for locator " + locator +
+ " as does not exist in invoker registry.");
+ return null;
+ }
+
+ ClientInvokerHolder holder = null;
+
// now look for specific invoker by configuration map
for(int x = 0; x < holderList.size(); x++)
{
@@ -573,30 +614,31 @@
}
}
- if(holder != null)
+ if (holder == null)
{
+ log.debug("Could not decrement client invoker counter for locator " + locator +
+ "as does not exist in invoker registry with matching configuraion map.");
+ return null;
+ }
+
+ ClientInvoker clientInvoker = null;
holder.decrementCount();
+
if(holder.getCount() == 0)
{
- // is the last reference to invoker
- // need to remove all references
clientInvoker = holder.getClientInvoker();
holderList.remove(holder);
if(holderList.isEmpty())
{
clientLocators.remove(locator);
}
- }
- }
- else
- {
- log.debug("Could not remove client invoker for locator (" + locator + ") " +
- "as does not exist in invoker registry with matching configuraion map.");
- }
+
+ log.debug("removed " + clientInvoker + " from registry");
}
else
{
- log.debug("Could not remove client invoker for locator (" + locator + ") as does not exist in invoker registry.");
+ log.debug("decremented " + holder.getClientInvoker() +
+ "'s count, current count " + holder.getCount());
}
return clientInvoker;
@@ -673,6 +715,11 @@
return isSSLSupported;
}
+ public String toString()
+ {
+ return "InvokerRegistry[" + Integer.toHexString(hashCode()) + "]";
+ }
+
private static class ClientInvokerHolder
{
private ClientInvoker invoker = null;
1.8.2.2 +11 -11 JBossRemoting/src/main/org/jboss/remoting/LeasePinger.java
(In the diff below, changes in quantity of whitespace are not shown.)
Index: LeasePinger.java
===================================================================
RCS file: /cvsroot/jboss/JBossRemoting/src/main/org/jboss/remoting/LeasePinger.java,v
retrieving revision 1.8.2.1
retrieving revision 1.8.2.2
diff -u -b -r1.8.2.1 -r1.8.2.2
--- LeasePinger.java 12 Dec 2006 03:48:04 -0000 1.8.2.1
+++ LeasePinger.java 10 Jan 2007 10:30:33 -0000 1.8.2.2
@@ -41,7 +41,7 @@
{
if(isTraceEnabled)
{
- log.trace("Starting lease timer for client invoker (session id " + invokerSessionId + ") with ping period of " + pingPeriod);
+ log.trace("Starting lease timer for client invoker with session ID " + invokerSessionId + " with ping period of " + pingPeriod);
}
timerTask = new LeaseTimerTask();
TimerUtil.schedule(timerTask, pingPeriod);
@@ -51,7 +51,7 @@
{
if(isTraceEnabled)
{
- log.trace("Stopping lease timer for client invoker (session id " + invokerSessionId + ")");
+ log.trace("Stopping lease timer for client invoker with session ID " + invokerSessionId);
}
if (timerTask != null)
{
@@ -90,7 +90,7 @@
clientSessionIds = clientSessionIds + h.getSessionId() + "\n";
}
}
- log.trace("Sending ping to server for client invoker (session id " + invokerSessionId + ". " +
+ log.trace("Sending ping to server for client invoker with session ID " + invokerSessionId + ". " +
"Currently managing lease for following clients:\n" + clientSessionIds);
} // end trace
@@ -102,7 +102,7 @@
}
catch (Throwable throwable)
{
- log.warn("Error sending lease ping to server for client invoker (session id " + invokerSessionId + ".");
+ log.warn("Error sending lease ping to server for client invoker with session ID " + invokerSessionId);
}
}
}
@@ -116,8 +116,8 @@
if(isTraceEnabled)
{
- log.trace("Adding new client to lease for client invoker (session id " + invokerSessionId + ") where " +
- "client session id is " + sessionId + " and lease period is " + leasePeriod);
+ log.trace("Adding new client to lease for client invoker with session ID " + invokerSessionId + " where " +
+ "client session ID is " + sessionId + " and lease period is " + leasePeriod);
}
ClientHolder newClient = new ClientHolder(sessionId, configuration, leasePeriod);
clients.put(sessionId, newClient);
@@ -144,7 +144,7 @@
if(isTraceEnabled)
{
- log.trace("Removing client (session id " + sessionId + ") from lease for client invoker (session id " + invokerSessionId + ")");
+ log.trace("Removing client with session ID " + sessionId + " from lease for client invoker with session ID " + invokerSessionId);
}
ClientHolder holder = (ClientHolder) clients.remove(sessionId);
if (holder != null)
@@ -157,17 +157,17 @@
client.invoke(new InvocationRequest(invokerSessionId, null, "$DISCONNECT$", clientMap, null, null));
if(isTraceEnabled)
{
- log.trace("Sent out disconnect message to server for lease tied to client session id " + sessionId);
+ log.trace("Sent out disconnect message to server for lease tied to client session ID " + sessionId);
}
}
catch (Throwable throwable)
{
- log.warn("Error sending disconnect for client lease where client session id is " + sessionId);
+ log.warn("Error sending disconnect for client lease where client session ID is " + sessionId);
}
}
else
{
- log.warn("Tried to remove lease for client (session id " + sessionId + "), but did not exist for client invoker lease (session id " + invokerSessionId);
+ log.warn("Tried to remove lease for client with session ID " + sessionId + ", but did not exist for client invoker lease (session ID " + invokerSessionId + ")");
}
if (clients.isEmpty())
@@ -175,7 +175,7 @@
isLastClientLease = true;
if(isTraceEnabled)
{
- log.trace("There are no more client leases tied to this client invoker's lease (session id " + invokerSessionId);
+ log.trace("There are no more client leases tied to this client invoker's lease (session ID " + invokerSessionId + ")");
}
}
else
1.7.2.1 +9 -4 JBossRemoting/src/main/org/jboss/remoting/MicroRemoteClientInvoker.java
(In the diff below, changes in quantity of whitespace are not shown.)
Index: MicroRemoteClientInvoker.java
===================================================================
RCS file: /cvsroot/jboss/JBossRemoting/src/main/org/jboss/remoting/MicroRemoteClientInvoker.java,v
retrieving revision 1.7
retrieving revision 1.7.2.1
diff -u -b -r1.7 -r1.7.2.1
--- MicroRemoteClientInvoker.java 21 Sep 2006 18:35:26 -0000 1.7
+++ MicroRemoteClientInvoker.java 10 Jan 2007 10:30:33 -0000 1.7.2.1
@@ -27,7 +27,7 @@
*
* @author <a href="mailto:jhaynie at vocalocity.net">Jeff Haynie</a>
* @author <a href="mailto:telrod at e2technologies.net">Tom Elrod</a>
- * @version $Revision: 1.7 $
+ * @version $Revision: 1.7.2.1 $
*/
public abstract class MicroRemoteClientInvoker extends AbstractInvoker implements ClientInvoker
{
@@ -67,7 +67,7 @@
if (isTraceEnabled)
{
- log.trace((++invokeCount) + ") invoking =>" + invocationReq + " with parameter: " + invocationReq.getParameter());
+ log.trace(this + "(" + (++invokeCount) + ") invoking =>" + invocationReq + " with parameter: " + invocationReq.getParameter());
}
Marshaller marshaller = getMarshaller();
@@ -288,10 +288,10 @@
*/
public synchronized void disconnect()
{
+ if (isTraceEnabled) { log.trace(this + " disconnecting ..."); }
+
if (connected)
{
- log.debug("disconnect called for: " + this);
-
connected = false;
handleDisconnect();
ClassLoader classLoader = getClassLoader();
@@ -299,6 +299,11 @@
{
((ClassByteClassLoader) classbyteloader).destroy();
}
+ if (isTraceEnabled) { log.trace(this + " disconnected"); }
+ }
+ else
+ {
+ if (isTraceEnabled) { log.trace(this + " is not connected!"); }
}
}
More information about the jboss-cvs-commits
mailing list