[jboss-cvs] JBossRemoting/src/main/org/jboss/remoting ...

Ron Sigal ron_sigal at yahoo.com
Wed Aug 22 22:56:28 EDT 2007


  User: rsigal  
  Date: 07/08/22 22:56:28

  Modified:    src/main/org/jboss/remoting  Tag: remoting_2_2_0_GA
                        LeasePinger.java
  Log:
  JBREM-783, JBREM-794:  Merged from 2_2_2_experimental branch: 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.8 +69 -205   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
  retrieving revision 1.8.2.12.2.8
  diff -u -b -r1.8.2.12.2.7 -r1.8.2.12.2.8
  --- LeasePinger.java	8 Aug 2007 05:47:22 -0000	1.8.2.12.2.7
  +++ LeasePinger.java	23 Aug 2007 02:56:28 -0000	1.8.2.12.2.8
  @@ -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)
  -   {
  -      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,28 +129,47 @@
               startPing();
            }
         }
  -
  -      sendClientPing();
      }
      
      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;
   
  +      if(trace) { log.trace(this + " removing client with session ID " + sessionID); }
  +
         ClientHolder holder = (ClientHolder)clients.remove(sessionID);
  -      if(trace) { log.trace(this + " removing client with session ID " + sessionID + " (" + clients.size() + ")"); }
         
  -      if (holder == null)
  +      if (holder != null)
  +      {
  +         // send disconnect for this client
  +         try
  +         {
  +            Map clientMap = new HashMap();
  +            clientMap.put(ClientHolder.CLIENT_HOLDER_KEY, holder);
  +            
  +            // If disconnectTimeout == 0, skip network i/o.
  +            if (disconnectTimeout != 0)
         {
  -         log.warn(this + " tried to remove lease for client with session ID "
  -                       + sessionID + ", but no such lease was found");
  -         return false;
  +               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.warn(this + " failed sending disconnect for client lease for " +
  +                  "client with session ID " + sessionID);
  +         }
  +      }
  +      else
  +      {
  +         log.warn(this + " tried to remove lease for client with session ID " + sessionID +
  +         ", but no such lease was found");
         }
         
         if (clients.isEmpty())
  @@ -209,93 +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);
  -               }
  -            }
  -            
  -            // 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.
  -         synchronized (disconnectLock)
  -         {
  -            if (stopped)
  -               return;
  -            
  -            if (stopping && closingClients.isEmpty())
  -            {
  -               stopped = true;
  -            } 
  -         }
  -         
  -         if (stopped)
  -         {
  -            disconnect();
  -         }
  -      }
  -   }
  -   
  -   public Map getClients()
  -   {
  -      return clients;
  -   }
  -
      public long getLeasePeriod(String sessionID)
      {
         if (timerTask == null)
  @@ -321,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 ------------------------------------------------------------------------------------
   
      
  @@ -387,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