[jboss-cvs] JBossAS SVN: r62764 - branches/JBoss_4_0_3_SP1_CP06/tomcat/src/main/org/jboss/web/tomcat/tc5/session.

jboss-cvs-commits at lists.jboss.org jboss-cvs-commits at lists.jboss.org
Wed May 2 18:09:53 EDT 2007


Author: bstansberry at jboss.com
Date: 2007-05-02 18:09:52 -0400 (Wed, 02 May 2007)
New Revision: 62764

Modified:
   branches/JBoss_4_0_3_SP1_CP06/tomcat/src/main/org/jboss/web/tomcat/tc5/session/AttributeBasedClusteredSession.java
   branches/JBoss_4_0_3_SP1_CP06/tomcat/src/main/org/jboss/web/tomcat/tc5/session/JBossCacheManager.java
Log:
[ASPATCH-208] Ensure the background thread doesn't override an active session

Modified: branches/JBoss_4_0_3_SP1_CP06/tomcat/src/main/org/jboss/web/tomcat/tc5/session/AttributeBasedClusteredSession.java
===================================================================
--- branches/JBoss_4_0_3_SP1_CP06/tomcat/src/main/org/jboss/web/tomcat/tc5/session/AttributeBasedClusteredSession.java	2007-05-02 22:08:16 UTC (rev 62763)
+++ branches/JBoss_4_0_3_SP1_CP06/tomcat/src/main/org/jboss/web/tomcat/tc5/session/AttributeBasedClusteredSession.java	2007-05-02 22:09:52 UTC (rev 62764)
@@ -104,7 +104,10 @@
    protected void populateAttributes()
    {
       Map map = proxy_.getAttributes(id);
-      if (map.size() != 0) attributes_ = map;
+      //if (map.size() != 0) attributes_ = map;
+      if (map == null)
+         map = new HashMap();
+      attributes_ = map;
    }
 
    // ----------------------------------------------------- Session Properties

Modified: branches/JBoss_4_0_3_SP1_CP06/tomcat/src/main/org/jboss/web/tomcat/tc5/session/JBossCacheManager.java
===================================================================
--- branches/JBoss_4_0_3_SP1_CP06/tomcat/src/main/org/jboss/web/tomcat/tc5/session/JBossCacheManager.java	2007-05-02 22:08:16 UTC (rev 62763)
+++ branches/JBoss_4_0_3_SP1_CP06/tomcat/src/main/org/jboss/web/tomcat/tc5/session/JBossCacheManager.java	2007-05-02 22:09:52 UTC (rev 62764)
@@ -464,39 +464,67 @@
    {
       // Will need to find from the underlying store
       List ids = proxy_.getNewSessionsInStore();
-      if(ids.size() ==0)
+      if(ids.size() > 0)
       {
-         Session[] sess = new Session[0];
-         sess = (Session[]) sessions_.values().toArray(sess);
-         return sess;
-      }
+         if(log_.isDebugEnabled()) {
+            log_.debug("findSessions: find ids from cache store: " + ids);
+         }
 
-      if(log_.isDebugEnabled()) {
-         log_.debug("findSessions: find ids from cache store: " + ids);
-      }
-
-
-      // Is there a better way to do this?
-      for(int i=0; i < ids.size(); i++) {
-         Session session = loadSession((String)ids.get(i));
-         if( session == null )
-         {
-            // This can happen now if the node has been removed before it has been called.
-
-            // Something is wrong with session. Should not have id but null session!
-//            log_.warn("Has session id: " +ids.get(i) + " but session is null. Will remove this from map");
-//            sessions_.remove(ids.get(i));
-//            proxy_.removeSession((String)ids.get(i));
-            continue;
+         // Is there a better way to do this?
+         for(int i=0; i < ids.size(); i++) {
+            importSession((String)ids.get(i));
          }
-         sessions_.put(ids.get(i), session);   // Populate local copy as well.
-      }
+      }      
 
       Session[] sess = new Session[0];
       sess = (Session[]) sessions_.values().toArray(sess);
       return sess;
    }
+   
+   private void importSession(String realId)
+   {
+      // A request may have already pulled the session into local
+      // management.  If so, don't do it again
+      if (sessions_.containsKey(realId))
+      {
+         if (log_.isTraceEnabled())
+         {
+            log_.trace("Skipping import of session " + realId + 
+                  " as it is already in local sessions map");
+         }
+      }
+      else
+      {
+         Session session = createEmptySession();
+         session = loadSession(realId, (ClusteredSession) session);
+            
+         if( session != null )
+         {
+            // Check again if a request has pulled it into local management
+            boolean storeIt = false;
+            synchronized (sessions_)
+            {
+               storeIt = !sessions_.containsKey(realId);
+               if (storeIt)
+               {
+                  sessions_.put(realId, session);   // Populate local copy as well.
 
+                  if (log_.isDebugEnabled())
+                  {
+                     log_.debug("importSession(): id= " + realId + ", session=" + session);
+                  }
+               }
+            }
+            
+            if (!storeIt & log_.isTraceEnabled())
+            {
+               log_.trace("Skipped import of session " + realId + 
+                     " as it is already in local sessions map");
+            }
+         }      
+      }
+   }
+
    public ClusteredSession[] findLocalSessions()
    {
       ClusteredSession[] sess = new ClusteredSession[0];
@@ -540,7 +568,7 @@
       // It's a bit ad-hoc to do it here. But since we currently call this when session expires ...
       expiredCounter_++;
       activeCounter_--;
-   }   
+   }      
 
    /**
     * Load a session from the distributed store
@@ -567,25 +595,66 @@
          session = (ClusteredSession) createEmptySession();
       }
       
-      session = proxy_.loadSession(realId, session);
+      session = loadSession(realId, session);
       
-      // Will need to initialize.
       if (session != null)
       {
-         session.initAfterLoad(this);
          /* Put the session into the local map or else other calls to find
          the session after the request completes will wipe out the dirty state
          due to any mods made by the web app.
          */
          sessions_.put(getRealId(session.getId()), session);
-//      long end = System.currentTimeMillis();
-//      stats_.updateLoadStats(id, (end - begin));
+
+         if (log_.isDebugEnabled())
+         {
+            log_.debug("loadSession(): id= " + realId + ", session=" + session);
+         }
       }
+         
+      return session; 
+   }
+   
+   protected ClusteredSession loadSession(String realId, 
+                                          ClusteredSession session)
+   {
+      boolean doTx = false;
+      try
+      {
+         // We need transaction so all the locks are held until all gets are done.
+         // Don't do anything if there is already transaction context associated with this thread.
+         if(tm.getTransaction() == null)
+            doTx = true;
 
-      if (log_.isDebugEnabled())
+         if(doTx)
+            tm.begin();
+         session = proxy_.loadSession(realId, session);
+         
+         // Will need to initialize.
+         if (session != null)
+         {
+            session.initAfterLoad(this);
+         }
+      }
+      catch (Exception ex)
       {
-         log_.debug("loadSession(): id= " + realId + ", session=" + session);
+         session = null;
+         
+         log_.error("importSession(): failed to import session " + realId, ex);
+         try
+         {
+            if(doTx)
+               tm.setRollbackOnly();
+         }
+         catch (Exception exn)
+         {
+            log_.error("Problem rolling back transaction", exn);
+         }
       }
+      finally
+      {
+         if(doTx)
+            endTransaction();
+      }
 
       return session;
    }




More information about the jboss-cvs-commits mailing list