On Tue, 18 Jun 2002 [EMAIL PROTECTED] wrote:

> Date: Tue, 18 Jun 2002 16:56:29 -0400 (EDT)
> From: [EMAIL PROTECTED]
> Reply-To: Tomcat Developers List <[EMAIL PROTECTED]>
> To: Tomcat Developers <[EMAIL PROTECTED]>
> 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:   <mailto:[EMAIL PROTECTED]>
> For additional commands, e-mail: <mailto:[EMAIL PROTECTED]>
>
>


--
To unsubscribe, e-mail:   <mailto:[EMAIL PROTECTED]>
For additional commands, e-mail: <mailto:[EMAIL PROTECTED]>

Reply via email to