You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by bo...@jadn.com on 2002/06/18 22:56:29 UTC

[PATCH] When Session Max reached, throw out oldest session

This PATCH implements the earlier PROPOSAL.  namely;

> PROPOSAL:  When Session Max reached, throw out oldest session
> 
> Currently tomcat ships with the maximum number of sessions set to
> unlimited.  They can expire by default after 30 minutes, however if
> too many sessions are created within the 30 minutes, you can run out
> of memory.
> 
> To prevent running out of memory, you might choose to limit the
> allowed number of active sessions.  If you use the default
> StanardManager (session manager) you can set the "maxActiveSessions"
> to effect a limit.  However if you exceed the number of allowed
> sessions, a RuntimeException (IllegalStateException) is thrown.
> 
> I propose two changes to reduce seeing these (IllegalStateException or
> OutOfMemory) exceptions for sessions;
> 
> 1. When the maximum number of sessions is reached, change
> StandardManager from throwing an IllegalStateException exception, to
> expiring the Least Recently Used (LRUMap) session.
> 
> 2. Instead of defaulting to an unlimited number of sessions (and
> getting visits from OutOfMemory), limit the number of sessions to
> 10000 by default.

This PATCH does 1 and 2.  It also fixes a problem with recycling
sessions (manager was set to null before session could be recycled.)

Can someone please review this?

Cheers,
-bob




Index: ManagerBase.java
===================================================================
RCS file: /home/cvspublic/jakarta-tomcat-4.0/catalina/src/share/org/apache/catalina/session/ManagerBase.java,v
retrieving revision 1.11
diff -u -r1.11 ManagerBase.java
--- ManagerBase.java    14 Jan 2002 23:38:03 -0000      1.11
+++ ManagerBase.java    18 Jun 2002 20:47:04 -0000
@@ -73,6 +73,8 @@
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.Random;
+import java.util.Map;
+import java.util.Collections;
 import org.apache.catalina.Container;
 import org.apache.catalina.Engine;
 import org.apache.catalina.Logger;
@@ -194,7 +196,7 @@
      * The set of currently active Sessions for this Manager, keyed by
      * session identifier.
      */
-    protected HashMap sessions = new HashMap();
+    protected Map sessions = Collections.synchronizedMap(new HashMap()) ;
 
 
     /**
@@ -496,11 +498,7 @@
      * @param session Session to be added
      */
     public void add(Session session) {
-
-        synchronized (sessions) {
-            sessions.put(session.getId(), session);
-        }
-
+        sessions.put(session.getId(), session);
     }
 
 
@@ -582,11 +580,8 @@
 
         if (id == null)
             return (null);
-        synchronized (sessions) {
-            Session session = (Session) sessions.get(id);
-            return (session);
-        }
 
+        return ((Session) sessions.get(id));
     }
 
 
@@ -595,14 +590,7 @@
      * If this Manager has no active Sessions, a zero-length array is returned.
      */
     public Session[] findSessions() {
-
-        Session results[] = null;
-        synchronized (sessions) {
-            results = new Session[sessions.size()];
-            results = (Session[]) sessions.values().toArray(results);
-        }
-        return (results);
-
+             return ((Session[]) sessions.values().toArray(new Session[0]));
     }
 
 
@@ -612,11 +600,7 @@
      * @param session Session to be removed
      */
     public void remove(Session session) {
-
-        synchronized (sessions) {
-            sessions.remove(session.getId());
-        }
-
+        sessions.remove(session.getId());
     }
 
 
Index: StandardManager.java
===================================================================
RCS file: /home/cvspublic/jakarta-tomcat-4.0/catalina/src/share/org/apache/catalina/session/StandardManager.java,v
retrieving revision 1.19
diff -u -r1.19 StandardManager.java
--- StandardManager.java        9 Jun 2002 02:19:43 -0000       1.19
+++ StandardManager.java        18 Jun 2002 20:47:06 -0000
@@ -80,6 +80,8 @@
 import java.io.ObjectStreamClass;
 import java.util.ArrayList;
 import java.util.Iterator;
+import java.util.HashMap;
+import java.util.Collections;
 import javax.servlet.ServletContext;
 import org.apache.catalina.Container;
 import org.apache.catalina.Context;
@@ -93,6 +95,7 @@
 import org.apache.catalina.Session;
 import org.apache.catalina.util.CustomObjectInputStream;
 import org.apache.catalina.util.LifecycleSupport;
+import org.apache.commons.collections.LRUMap;
 
 
 /**
@@ -138,7 +141,7 @@
     /**
      * The maximum number of active Sessions allowed, or -1 for no limit.
      */
-    private int maxActiveSessions = -1;
+    private int maxActiveSessions = 10000;
 
 
     /**
@@ -274,6 +277,13 @@
                                    new Integer(oldMaxActiveSessions),
                                    new Integer(this.maxActiveSessions));
 
+        expireAll();
+
+        if ( max <= 0 )
+            sessions = Collections.synchronizedMap(new HashMap()) ;
+        else
+            sessions = Collections.synchronizedMap(new LocalLRUMap(max) );
+
     }
 
 
@@ -314,29 +324,6 @@
 
     // --------------------------------------------------------- Public Methods
 
-
-    /**
-     * Construct and return a new session object, based on the default
-     * settings specified by this Manager's properties.  The session
-     * id will be assigned by this method, and available via the getId()
-     * method of the returned session.  If a new session cannot be created
-     * for any reason, return <code>null</code>.
-     *
-     * @exception IllegalStateException if a new session cannot be
-     *  instantiated for any reason
-     */
-    public Session createSession() {
-
-        if ((maxActiveSessions >= 0) &&
-          (sessions.size() >= maxActiveSessions))
-            throw new IllegalStateException
-                (sm.getString("standardManager.createSession.ise"));
-
-        return (super.createSession());
-
-    }
-
-
     /**
      * Load any currently active sessions that were previously unloaded
      * to the appropriate persistence mechanism, if any.  If persistence is not
@@ -353,7 +340,10 @@
 
         // Initialize our internal data structures
         recycled.clear();
-        sessions.clear();
+        if ( maxActiveSessions <= 0 )
+            sessions = Collections.synchronizedMap(new HashMap() );
+        else
+            sessions = Collections.synchronizedMap(new LocalLRUMap(maxActiveSessions) );
 
         // Open an input stream to the specified pathname, if any
         File file = file();
@@ -414,7 +404,7 @@
                     ((StandardSession) session).activate();
                 }
             } catch (ClassNotFoundException e) {
-              log(sm.getString("standardManager.loading.cnfe", e), e);
+                log(sm.getString("standardManager.loading.cnfe", e), e);
                 if (ois != null) {
                     try {
                         ois.close();
@@ -425,7 +415,7 @@
                 }
                 throw e;
             } catch (IOException e) {
-              log(sm.getString("standardManager.loading.ioe", e), e);
+                log(sm.getString("standardManager.loading.ioe", e), e);
                 if (ois != null) {
                     try {
                         ois.close();
@@ -492,7 +482,6 @@
         }
 
         // Write the number of active sessions, followed by the details
-        ArrayList list = new ArrayList();
         synchronized (sessions) {
             if (debug >= 1)
                 log("Unloading " + sessions.size() + " sessions");
@@ -502,7 +491,6 @@
                 while (elements.hasNext()) {
                     StandardSession session =
                         (StandardSession) elements.next();
-                    list.add(session);
                     ((StandardSession) session).passivate();
                     session.writeObjectData(oos);
                 }
@@ -537,18 +525,7 @@
             throw e;
         }
 
-        // Expire all the sessions we just wrote
-        if (debug >= 1)
-            log("Expiring " + list.size() + " persisted sessions");
-        Iterator expires = list.iterator();
-        while (expires.hasNext()) {
-            StandardSession session = (StandardSession) expires.next();
-            try {
-                session.expire(false);
-            } catch (Throwable t) {
-                ;
-            }
-        }
+        expireAll();
 
         if (debug >= 1)
             log("Unloading complete");
@@ -664,18 +641,7 @@
             log(sm.getString("standardManager.managerUnload"), e);
         }
 
-        // Expire all active sessions
-        Session sessions[] = findSessions();
-        for (int i = 0; i < sessions.length; i++) {
-            StandardSession session = (StandardSession) sessions[i];
-            if (!session.isValid())
-                continue;
-            try {
-                session.expire();
-            } catch (Throwable t) {
-                ;
-            }
-        }
+        expireAll();
 
         // Require a new random number generator if we are restarted
         this.random = null;
@@ -714,6 +680,31 @@
 
     // -------------------------------------------------------- Private Methods
 
+    /**
+     * extention of commons LRUMap, handles expiration and works around
+     * an LRU bug
+     */
+    private class LocalLRUMap extends LRUMap {
+       LocalLRUMap(int max) {
+           super(max);
+       }
+
+       protected void processRemovedLRU(Object key, Object value){
+           ((Session)value).expire();
+           log("StandardManager: WARNING Max sessions reached, expiring oldest key:"+key);
+       }
+
+       // collections 2.0 has a bug in it - fixed in the latest CVS
+       public Object get(Object key){
+           
+           // put() it to move it to the top of the LRU
+           if ( containsKey( key ) ){
+               put( key, super.get(key));
+           }
+           return super.get(key);
+       }
+    };
+
 
     /**
      * Return a File object representing the pathname to our
@@ -823,6 +814,27 @@
 
         thread = null;
 
+    }
+
+
+    /**
+     * Expire all active sessions
+     */
+    private void expireAll(){
+
+        Session list[] = findSessions();
+        for (int i = 0; i < list.length; i++) {
+            StandardSession session = (StandardSession) list[i];
+            if (!session.isValid())
+                continue;
+            try {
+                session.expire();
+            } catch (Throwable t) {
+                ;
+            }
+        }
+
+        sessions.clear();
     }
 
 

--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


Re: [PATCH] When Session Max reached, throw out oldest session

Posted by Remy Maucherat <re...@apache.org>.
Craig R. McClanahan wrote:
> 
> On Tue, 18 Jun 2002 bob@jadn.com wrote:
> 
> 
>>Date: Tue, 18 Jun 2002 16:56:29 -0400 (EDT)
>>From: bob@jadn.com
>>Reply-To: Tomcat Developers List <to...@jakarta.apache.org>
>>To: Tomcat Developers <to...@jakarta.apache.org>
>>Subject: [PATCH] When Session Max reached, throw out oldest session
>>
>>
>>This PATCH implements the earlier PROPOSAL.  namely;
>>
>>
>>>PROPOSAL:  When Session Max reached, throw out oldest session
>>>
>>>Currently tomcat ships with the maximum number of sessions set to
>>>unlimited.  They can expire by default after 30 minutes, however if
>>>too many sessions are created within the 30 minutes, you can run out
>>>of memory.
>>>
>>>To prevent running out of memory, you might choose to limit the
>>>allowed number of active sessions.  If you use the default
>>>StanardManager (session manager) you can set the "maxActiveSessions"
>>>to effect a limit.  However if you exceed the number of allowed
>>>sessions, a RuntimeException (IllegalStateException) is thrown.
>>>
>>>I propose two changes to reduce seeing these (IllegalStateException or
>>>OutOfMemory) exceptions for sessions;
>>>
>>>1. When the maximum number of sessions is reached, change
>>>StandardManager from throwing an IllegalStateException exception, to
>>>expiring the Least Recently Used (LRUMap) session.
>>>
>>>2. Instead of defaulting to an unlimited number of sessions (and
>>>getting visits from OutOfMemory), limit the number of sessions to
>>>10000 by default.
>>
>>This PATCH does 1 and 2.  It also fixes a problem with recycling
>>sessions (manager was set to null before session could be recycled.)
>>
>>Can someone please review this?
>>
> 
> 
> I'm neutral on #2 (anyone who is really going to have that many sessions
> is likely to be tuning lots of configuration parameters anyway), but I
> don't think #1 is a good idea.
> 
> Implementing it would be like running Jon's night club with the rule that,
> when a new person came along, had the bouncer going in to kick out the
> person whose beer is the warmest.  It's probably better customer service
> for the newcomer to wait until someone leaves voluntarily, which is how
> the current algorithm works.  :-)
> 
> More seriously, you could force a DOS attack against an app that worked
> this way, simply by continuously causing new sessions to be created.
> Every real user that tried would get in, but only for one request.  (You
> can disrupt new users for an app based on the current algorithm, but you
> can't cause any existing user to get kicked out.)

Ok, so maybe the behavior is bad in the default case, but the fact is 
the server may start working altogether without that (optional) limit. I 
like the idea of losing one session a bit better. The administrator 
would have to tweak that so that he would set the maximum value which 
would allow its server to run.

OTOH, we could change it, and have the default be to have no limit.

Remy


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


Re: [PATCH] When Session Max reached, throw out oldest session

Posted by "Craig R. McClanahan" <cr...@apache.org>.

On Tue, 18 Jun 2002 bob@jadn.com wrote:

> Date: Tue, 18 Jun 2002 16:56:29 -0400 (EDT)
> From: bob@jadn.com
> Reply-To: Tomcat Developers List <to...@jakarta.apache.org>
> To: Tomcat Developers <to...@jakarta.apache.org>
> Subject: [PATCH] When Session Max reached, throw out oldest session
>
>
> This PATCH implements the earlier PROPOSAL.  namely;
>
> > PROPOSAL:  When Session Max reached, throw out oldest session
> >
> > Currently tomcat ships with the maximum number of sessions set to
> > unlimited.  They can expire by default after 30 minutes, however if
> > too many sessions are created within the 30 minutes, you can run out
> > of memory.
> >
> > To prevent running out of memory, you might choose to limit the
> > allowed number of active sessions.  If you use the default
> > StanardManager (session manager) you can set the "maxActiveSessions"
> > to effect a limit.  However if you exceed the number of allowed
> > sessions, a RuntimeException (IllegalStateException) is thrown.
> >
> > I propose two changes to reduce seeing these (IllegalStateException or
> > OutOfMemory) exceptions for sessions;
> >
> > 1. When the maximum number of sessions is reached, change
> > StandardManager from throwing an IllegalStateException exception, to
> > expiring the Least Recently Used (LRUMap) session.
> >
> > 2. Instead of defaulting to an unlimited number of sessions (and
> > getting visits from OutOfMemory), limit the number of sessions to
> > 10000 by default.
>
> This PATCH does 1 and 2.  It also fixes a problem with recycling
> sessions (manager was set to null before session could be recycled.)
>
> Can someone please review this?
>

I'm neutral on #2 (anyone who is really going to have that many sessions
is likely to be tuning lots of configuration parameters anyway), but I
don't think #1 is a good idea.

Implementing it would be like running Jon's night club with the rule that,
when a new person came along, had the bouncer going in to kick out the
person whose beer is the warmest.  It's probably better customer service
for the newcomer to wait until someone leaves voluntarily, which is how
the current algorithm works.  :-)

More seriously, you could force a DOS attack against an app that worked
this way, simply by continuously causing new sessions to be created.
Every real user that tried would get in, but only for one request.  (You
can disrupt new users for an app based on the current algorithm, but you
can't cause any existing user to get kicked out.)

> Cheers,
> -bob
>

Craig


>
>
>
> Index: ManagerBase.java
> ===================================================================
> RCS file: /home/cvspublic/jakarta-tomcat-4.0/catalina/src/share/org/apache/catalina/session/ManagerBase.java,v
> retrieving revision 1.11
> diff -u -r1.11 ManagerBase.java
> --- ManagerBase.java    14 Jan 2002 23:38:03 -0000      1.11
> +++ ManagerBase.java    18 Jun 2002 20:47:04 -0000
> @@ -73,6 +73,8 @@
>  import java.util.ArrayList;
>  import java.util.HashMap;
>  import java.util.Random;
> +import java.util.Map;
> +import java.util.Collections;
>  import org.apache.catalina.Container;
>  import org.apache.catalina.Engine;
>  import org.apache.catalina.Logger;
> @@ -194,7 +196,7 @@
>       * The set of currently active Sessions for this Manager, keyed by
>       * session identifier.
>       */
> -    protected HashMap sessions = new HashMap();
> +    protected Map sessions = Collections.synchronizedMap(new HashMap()) ;
>
>
>      /**
> @@ -496,11 +498,7 @@
>       * @param session Session to be added
>       */
>      public void add(Session session) {
> -
> -        synchronized (sessions) {
> -            sessions.put(session.getId(), session);
> -        }
> -
> +        sessions.put(session.getId(), session);
>      }
>
>
> @@ -582,11 +580,8 @@
>
>          if (id == null)
>              return (null);
> -        synchronized (sessions) {
> -            Session session = (Session) sessions.get(id);
> -            return (session);
> -        }
>
> +        return ((Session) sessions.get(id));
>      }
>
>
> @@ -595,14 +590,7 @@
>       * If this Manager has no active Sessions, a zero-length array is returned.
>       */
>      public Session[] findSessions() {
> -
> -        Session results[] = null;
> -        synchronized (sessions) {
> -            results = new Session[sessions.size()];
> -            results = (Session[]) sessions.values().toArray(results);
> -        }
> -        return (results);
> -
> +             return ((Session[]) sessions.values().toArray(new Session[0]));
>      }
>
>
> @@ -612,11 +600,7 @@
>       * @param session Session to be removed
>       */
>      public void remove(Session session) {
> -
> -        synchronized (sessions) {
> -            sessions.remove(session.getId());
> -        }
> -
> +        sessions.remove(session.getId());
>      }
>
>
> Index: StandardManager.java
> ===================================================================
> RCS file: /home/cvspublic/jakarta-tomcat-4.0/catalina/src/share/org/apache/catalina/session/StandardManager.java,v
> retrieving revision 1.19
> diff -u -r1.19 StandardManager.java
> --- StandardManager.java        9 Jun 2002 02:19:43 -0000       1.19
> +++ StandardManager.java        18 Jun 2002 20:47:06 -0000
> @@ -80,6 +80,8 @@
>  import java.io.ObjectStreamClass;
>  import java.util.ArrayList;
>  import java.util.Iterator;
> +import java.util.HashMap;
> +import java.util.Collections;
>  import javax.servlet.ServletContext;
>  import org.apache.catalina.Container;
>  import org.apache.catalina.Context;
> @@ -93,6 +95,7 @@
>  import org.apache.catalina.Session;
>  import org.apache.catalina.util.CustomObjectInputStream;
>  import org.apache.catalina.util.LifecycleSupport;
> +import org.apache.commons.collections.LRUMap;
>
>
>  /**
> @@ -138,7 +141,7 @@
>      /**
>       * The maximum number of active Sessions allowed, or -1 for no limit.
>       */
> -    private int maxActiveSessions = -1;
> +    private int maxActiveSessions = 10000;
>
>
>      /**
> @@ -274,6 +277,13 @@
>                                     new Integer(oldMaxActiveSessions),
>                                     new Integer(this.maxActiveSessions));
>
> +        expireAll();
> +
> +        if ( max <= 0 )
> +            sessions = Collections.synchronizedMap(new HashMap()) ;
> +        else
> +            sessions = Collections.synchronizedMap(new LocalLRUMap(max) );
> +
>      }
>
>
> @@ -314,29 +324,6 @@
>
>      // --------------------------------------------------------- Public Methods
>
> -
> -    /**
> -     * Construct and return a new session object, based on the default
> -     * settings specified by this Manager's properties.  The session
> -     * id will be assigned by this method, and available via the getId()
> -     * method of the returned session.  If a new session cannot be created
> -     * for any reason, return <code>null</code>.
> -     *
> -     * @exception IllegalStateException if a new session cannot be
> -     *  instantiated for any reason
> -     */
> -    public Session createSession() {
> -
> -        if ((maxActiveSessions >= 0) &&
> -          (sessions.size() >= maxActiveSessions))
> -            throw new IllegalStateException
> -                (sm.getString("standardManager.createSession.ise"));
> -
> -        return (super.createSession());
> -
> -    }
> -
> -
>      /**
>       * Load any currently active sessions that were previously unloaded
>       * to the appropriate persistence mechanism, if any.  If persistence is not
> @@ -353,7 +340,10 @@
>
>          // Initialize our internal data structures
>          recycled.clear();
> -        sessions.clear();
> +        if ( maxActiveSessions <= 0 )
> +            sessions = Collections.synchronizedMap(new HashMap() );
> +        else
> +            sessions = Collections.synchronizedMap(new LocalLRUMap(maxActiveSessions) );
>
>          // Open an input stream to the specified pathname, if any
>          File file = file();
> @@ -414,7 +404,7 @@
>                      ((StandardSession) session).activate();
>                  }
>              } catch (ClassNotFoundException e) {
> -              log(sm.getString("standardManager.loading.cnfe", e), e);
> +                log(sm.getString("standardManager.loading.cnfe", e), e);
>                  if (ois != null) {
>                      try {
>                          ois.close();
> @@ -425,7 +415,7 @@
>                  }
>                  throw e;
>              } catch (IOException e) {
> -              log(sm.getString("standardManager.loading.ioe", e), e);
> +                log(sm.getString("standardManager.loading.ioe", e), e);
>                  if (ois != null) {
>                      try {
>                          ois.close();
> @@ -492,7 +482,6 @@
>          }
>
>          // Write the number of active sessions, followed by the details
> -        ArrayList list = new ArrayList();
>          synchronized (sessions) {
>              if (debug >= 1)
>                  log("Unloading " + sessions.size() + " sessions");
> @@ -502,7 +491,6 @@
>                  while (elements.hasNext()) {
>                      StandardSession session =
>                          (StandardSession) elements.next();
> -                    list.add(session);
>                      ((StandardSession) session).passivate();
>                      session.writeObjectData(oos);
>                  }
> @@ -537,18 +525,7 @@
>              throw e;
>          }
>
> -        // Expire all the sessions we just wrote
> -        if (debug >= 1)
> -            log("Expiring " + list.size() + " persisted sessions");
> -        Iterator expires = list.iterator();
> -        while (expires.hasNext()) {
> -            StandardSession session = (StandardSession) expires.next();
> -            try {
> -                session.expire(false);
> -            } catch (Throwable t) {
> -                ;
> -            }
> -        }
> +        expireAll();
>
>          if (debug >= 1)
>              log("Unloading complete");
> @@ -664,18 +641,7 @@
>              log(sm.getString("standardManager.managerUnload"), e);
>          }
>
> -        // Expire all active sessions
> -        Session sessions[] = findSessions();
> -        for (int i = 0; i < sessions.length; i++) {
> -            StandardSession session = (StandardSession) sessions[i];
> -            if (!session.isValid())
> -                continue;
> -            try {
> -                session.expire();
> -            } catch (Throwable t) {
> -                ;
> -            }
> -        }
> +        expireAll();
>
>          // Require a new random number generator if we are restarted
>          this.random = null;
> @@ -714,6 +680,31 @@
>
>      // -------------------------------------------------------- Private Methods
>
> +    /**
> +     * extention of commons LRUMap, handles expiration and works around
> +     * an LRU bug
> +     */
> +    private class LocalLRUMap extends LRUMap {
> +       LocalLRUMap(int max) {
> +           super(max);
> +       }
> +
> +       protected void processRemovedLRU(Object key, Object value){
> +           ((Session)value).expire();
> +           log("StandardManager: WARNING Max sessions reached, expiring oldest key:"+key);
> +       }
> +
> +       // collections 2.0 has a bug in it - fixed in the latest CVS
> +       public Object get(Object key){
> +
> +           // put() it to move it to the top of the LRU
> +           if ( containsKey( key ) ){
> +               put( key, super.get(key));
> +           }
> +           return super.get(key);
> +       }
> +    };
> +
>
>      /**
>       * Return a File object representing the pathname to our
> @@ -823,6 +814,27 @@
>
>          thread = null;
>
> +    }
> +
> +
> +    /**
> +     * Expire all active sessions
> +     */
> +    private void expireAll(){
> +
> +        Session list[] = findSessions();
> +        for (int i = 0; i < list.length; i++) {
> +            StandardSession session = (StandardSession) list[i];
> +            if (!session.isValid())
> +                continue;
> +            try {
> +                session.expire();
> +            } catch (Throwable t) {
> +                ;
> +            }
> +        }
> +
> +        sessions.clear();
>      }
>
>
>
> --
> To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
> For additional commands, e-mail: <ma...@jakarta.apache.org>
>
>


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>