[jboss-cvs] JBossRemoting/src/main/org/jboss/remoting ...
Ron Sigal
ron_sigal at yahoo.com
Mon Aug 20 01:31:14 EDT 2007
User: rsigal
Date: 07/08/20 01:31:14
Modified: src/main/org/jboss/remoting Tag:
remoting_2_2_2_experimental LeasePinger.java
Log:
JBREM-783, JBREM-794: Reverted to version before changes made for JBREM-783; (2) moved startPing() in addClient().
Revision Changes Path
No revision
No revision
1.8.2.12.2.7.2.2 +68 -244 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.12.2.7.2.1
retrieving revision 1.8.2.12.2.7.2.2
diff -u -b -r1.8.2.12.2.7.2.1 -r1.8.2.12.2.7.2.2
--- LeasePinger.java 18 Aug 2007 00:57:11 -0000 1.8.2.12.2.7.2.1
+++ LeasePinger.java 20 Aug 2007 05:31:14 -0000 1.8.2.12.2.7.2.2
@@ -40,79 +40,66 @@
private String invokerSessionID = null;
private Map clients = new ConcurrentHashMap();
- private Map closingClients = new ConcurrentHashMap();
private TimerTask timerTask = null;
private long pingPeriod = -1;
private int disconnectTimeout = DEFAULT_DISCONNECT_TIMEOUT;
- private boolean started;
- private boolean stopping;
- private boolean stopped;
- private Object disconnectLock = new Object();
// Constructors ---------------------------------------------------------------------------------
- public LeasePinger(ClientInvoker invoker, String invokerSessionID)
- {
- this(invoker, invokerSessionID, DEFAULT_LEASE_PERIOD);
- }
-
public LeasePinger(ClientInvoker invoker, String invokerSessionID, long defaultLeasePeriod)
{
this.invoker = invoker;
this.invokerSessionID = invokerSessionID;
this.pingPeriod = defaultLeasePeriod;
this.defaultPingPeriod = defaultLeasePeriod;
- log.debug(this + " created LeasePinger: " + invokerSessionID);
}
// Public ---------------------------------------------------------------------------------------
- public void setDefaultLeasePeriod(long defaultLeasePeriod)
- {
- this.defaultPingPeriod = defaultLeasePeriod;
- if (defaultLeasePeriod < this.pingPeriod)
- this.pingPeriod = defaultLeasePeriod;
- }
-
public void startPing()
{
if(trace) { log.trace(this + " starting lease timer with ping period of " + pingPeriod); }
timerTask = new LeaseTimerTask(this);
timer.schedule(timerTask, pingPeriod, pingPeriod);
- started = true;
}
- public boolean isStarted()
+ public void stopPing()
{
- return started;
- }
+ if(trace) { log.trace(this + " stopping lease timer"); }
- public void stopPing()
+ if (timerTask != null)
{
- // The division of removeClient() (synchronized by MicroRemoteClientInvoker)
- // into removeClient() (synchronized by MicroRemoteClientInvoker) and
- // disconnectClient() (not synchronized) has created some nondeterminism.
- // In particular, it allows the following scenario:
- //
- // 1. Thread A calls removeClient() for the next to the last client.
- // 2. Thread B calls removeClient() for the last client.
- // 3. Thread B calls disconnectClient() for the last client.
- // 4. Thread B calls stopPing(), destroying the Lease on the server side.
- // 5. Thread A calls disconnectClient() for the next to the last client.
- //
- // However, Thread A's call to disconnectClient() would fail because the
- // server has already destroyed the lease.
- //
- // The solution is for stopPing() to simply set a flag, stopping, to true.
- // The first thread to enter disconnectClient() and determine that stopping
- // is true and that it has disconnected the last client will then call
- // disconnect(), which tells the server to destroy the Lease. (Note that
- // this determination requires some synchronization to avoid the possibility
- // that two or more threads try to contact the server.)
+ timerTask.cancel();
+ timerTask = null;
- stopping = true;
+ try
+ {
+ // sending request map with no ClientHolders will indicate to server
+ // that is full disconnect (for client invoker)
+ HashMap metadata = null;
+
+ // If disconnectTimeout == 0, skip network i/o.
+ if (disconnectTimeout != 0)
+ {
+ if (disconnectTimeout > 0)
+ {
+ metadata = new HashMap(1);
+ metadata.put(ServerInvoker.TIMEOUT, Integer.toString(disconnectTimeout));
+ }
+ InvocationRequest ir =
+ new InvocationRequest(invokerSessionID, null, "$DISCONNECT$", metadata, null, null);
+ invoker.invoke(ir);
+ }
+ }
+ catch (Throwable throwable)
+ {
+ RuntimeException e = new RuntimeException("Error tearing down lease with server.");
+ e.initCause(throwable);
+ throw e;
+ }
+ }
}
public void addClient(String sessionID, Map configuration, long leasePeriod)
@@ -126,14 +113,8 @@
ClientHolder newClient = new ClientHolder(sessionID, configuration, leasePeriod);
clients.put(sessionID, newClient);
- }
- public void connectClient(long leasePeriod, String sessionID)
- {
- if (leasePeriod <= 0)
- {
- leasePeriod = defaultPingPeriod;
- }
+ sendClientPing();
// if new client lease period is less than the current ping period, need to refresh to new one
if (leasePeriod < pingPeriod)
@@ -148,56 +129,47 @@
startPing();
}
}
+ }
- if(trace) { log.trace(this + " connecting client with session ID " + sessionID); }
+ public boolean removeClient(String sessionID)
+ {
+ boolean isLastClientLease = false;
- ClientHolder holder = (ClientHolder) clients.get(sessionID);
+ if(trace) { log.trace(this + " removing client with session ID " + sessionID); }
+
+ ClientHolder holder = (ClientHolder)clients.remove(sessionID);
if (holder != null)
{
- // send connect for this client
+ // send disconnect for this client
+ try
+ {
Map clientMap = new HashMap();
clientMap.put(ClientHolder.CLIENT_HOLDER_KEY, holder);
- if (trace) log.trace(this + " requesting client lease connect: " + invokerSessionID + ": " + holder);
- InvocationRequest ir = new InvocationRequest(invokerSessionID, null, "$PING$",
- clientMap, null, null);
- try
+ // If disconnectTimeout == 0, skip network i/o.
+ if (disconnectTimeout != 0)
{
+ if (disconnectTimeout > 0)
+ clientMap.put(ServerInvoker.TIMEOUT, Integer.toString(disconnectTimeout));
+
+ InvocationRequest ir = new InvocationRequest(invokerSessionID, null, "$DISCONNECT$",
+ clientMap, null, null);
invoker.invoke(ir);
+
if(trace) { log.trace(this + " sent out disconnect message to server for lease tied to client with session ID " + sessionID); }
}
+ }
catch (Throwable throwable)
{
- log.error(this + " failed sending connect for client lease for " +
- "client with session ID " + sessionID, throwable);
+ log.warn(this + " failed sending disconnect for client lease for " +
+ "client with session ID " + sessionID);
}
}
else
{
- log.warn(this + " tried to connect client with session ID "
- + sessionID + ", but no such lease was found");
- }
- }
-
- public boolean removeClient(String sessionID)
- {
- // removeClient() has been reorganized to support more nimble use of
- // synchronization blocks - see JBREM-783.
-
- // In particular, the network i/o formerly found in removeClient() has been
- // moved to the new method disconnectClient().
-
- boolean isLastClientLease = false;
-
- ClientHolder holder = (ClientHolder)clients.remove(sessionID);
- if(trace) { log.trace(this + " removing client with session ID " + sessionID + " (" + clients.size() + ")"); }
-
- if (holder == null)
- {
- log.warn(this + " tried to remove lease for client with session ID "
- + sessionID + ", but no such lease was found");
- return false;
+ log.warn(this + " tried to remove lease for client with session ID " + sessionID +
+ ", but no such lease was found");
}
if (clients.isEmpty())
@@ -237,109 +209,9 @@
}
}
-
- synchronized (disconnectLock)
- {
- closingClients.put(sessionID, holder);
- }
-
return isLastClientLease;
}
- public void disconnectClient(String sessionID)
- {
- try
- {
- if(trace) { log.trace(this + " disconnecting client with session ID " + sessionID); }
-
- ClientHolder holder = (ClientHolder)closingClients.get(sessionID);
-
- if (holder != null)
- {
- // send disconnect for this client
- Map clientMap = new HashMap();
- clientMap.put(ClientHolder.CLIENT_HOLDER_KEY, holder);
-
- // If disconnectTimeout == 0, skip network i/o.
- if (disconnectTimeout != 0)
- {
- if (disconnectTimeout > 0)
- clientMap.put(ServerInvoker.TIMEOUT, Integer.toString(disconnectTimeout));
-
- if (trace) log.trace(this + " requesting client lease disconnect: " + invokerSessionID);
- InvocationRequest ir = new InvocationRequest(invokerSessionID, null, "$DISCONNECT$",
- clientMap, null, null);
- try
- {
- invoker.invoke(ir);
- if(trace) { log.trace(this + " sent out disconnect message to server for lease tied to client with session ID " + sessionID); }
- }
- catch (Throwable throwable)
- {
- log.error(this + " failed sending disconnect for client lease for " +
- "client with session ID " + sessionID, throwable);
- }
- finally
- {
- // closingClients will be empty only after an attempt has been made to
- // disconnect all clients.
- synchronized (disconnectLock)
- {
- closingClients.remove(sessionID);
- }
- }
- }
- }
- else
- {
- log.warn(this + " tried to disconnect client with session ID "
- + sessionID + ", but no such lease was found");
- }
- }
- finally
- {
- // The first thread to enter this synchronization block and find that
- // stopping == true and closingClients is empty will tell the server
- // to destroy the Lease. Any other threads that enter the block
- // under the same conditions will find stopped == true and will
- // return without attempting to contact the server.
-
- // It's still possible for the following sequence to occur:
- // 1. thread A enters synchronized block, finds condition
- // stopping && closingClients.isEmpty() false
- // 2. thread B enters synchronized block, finds condition
- // stopping && closingClients.isEmpty() true
- // 3. thread A sees stopped == true and calls disconnect
- // 4. thread B sees stopped == true and calls disconect
- //
- // Therefore, the variable localStopped is used, so that only the
- // thread that set stopped to true will call disconnect.
-
- boolean localStopped = false;
- synchronized (disconnectLock)
- {
- if (stopped)
- return;
-
- if (stopping && closingClients.isEmpty())
- {
- stopped = true;
- localStopped = true;
- }
- }
-
- if (localStopped)
- {
- disconnect();
- }
- }
- }
-
- public Map getClients()
- {
- return clients;
- }
-
public long getLeasePeriod(String sessionID)
{
if (timerTask == null)
@@ -365,18 +237,6 @@
// Package protected ----------------------------------------------------------------------------
- /**
- * Simply removes references to all clients. Should only be called on a LeasePinger
- * which was never started.
- */
- protected void purgeClients()
- {
- if (!started)
- {
- clients.clear();
- }
- }
-
// Protected ------------------------------------------------------------------------------------
@@ -413,8 +273,12 @@
"for following clients:\n" + sb.toString());
}
+ Map clientsClone = new ConcurrentHashMap(clients);
+ Map requestClients = new ConcurrentHashMap();
+ requestClients.put(ClientHolder.CLIENT_HOLDER_KEY, clientsClone);
+
InvocationRequest ir =
- new InvocationRequest(invokerSessionID, null, "$PING$", null, null, null);
+ new InvocationRequest(invokerSessionID, null, "$PING$", requestClients, null, null);
invoker.invoke(ir);
@@ -427,46 +291,6 @@
}
}
- private void disconnect()
- {
- if(trace) { log.trace(this + " destroying lease"); }
-
- if (timerTask != null)
- {
- timerTask.cancel();
- timerTask = null;
- }
-
- try
- {
- // sending request map with no ClientHolders will indicate to server
- // that is full disconnect (for client invoker)
- HashMap metadata = null;
-
- // If disconnectTimeout == 0, skip network i/o.
- if (disconnectTimeout != 0)
- {
- if (disconnectTimeout > 0)
- {
- metadata = new HashMap(1);
- metadata.put(ServerInvoker.TIMEOUT, Integer.toString(disconnectTimeout));
- }
-
- if (trace) log.trace("requesting invoker lease disconnect: " + invokerSessionID);
- InvocationRequest ir =
- new InvocationRequest(invokerSessionID, null, "$DISCONNECT$", metadata, null, null);
- invoker.invoke(ir);
- if (trace) log.trace(this + " destroyed lease");
- }
- }
- catch (Throwable throwable)
- {
- RuntimeException e = new RuntimeException("Error tearing down lease with server.");
- e.initCause(throwable);
- throw e;
- }
- }
-
// Inner classes --------------------------------------------------------------------------------
static private class LeaseTimerTask extends TimerTask
More information about the jboss-cvs-commits
mailing list