[jboss-cvs] jboss-seam/src/main/org/jboss/seam/core ...

[Gavin King]

  User: gavin   
  Date: 06/10/12 22:24:30

  Modified:    src/main/org/jboss/seam/core   ConversationEntry.java
                        Manager.java
  Log:
  better thought-out locking model
  
  Revision  Changes    Path
  1.23      +15 -10    jboss-seam/src/main/org/jboss/seam/core/ConversationEntry.java
  
  (In the diff below, changes in quantity of whitespace are not shown.)
  
  Index: ConversationEntry.java
  ===================================================================
  RCS file: /cvsroot/jboss/jboss-seam/src/main/org/jboss/seam/core/ConversationEntry.java,v
  retrieving revision 1.22
  retrieving revision 1.23
  diff -u -b -r1.22 -r1.23
  --- ConversationEntry.java	13 Oct 2006 01:27:31 -0000	1.22
  +++ ConversationEntry.java	13 Oct 2006 02:24:30 -0000	1.23
  @@ -7,8 +7,8 @@
   import java.util.Collections;
   import java.util.Date;
   import java.util.List;
  -import java.util.concurrent.Semaphore;
   import java.util.concurrent.TimeUnit;
  +import java.util.concurrent.locks.ReentrantLock;
   
   /**
    * Metadata about an active conversation. Also used
  @@ -32,7 +32,7 @@
      
      private ConversationEntries parent;
      
  -   private Semaphore semaphore = new Semaphore(1,true);
  +   private ReentrantLock lock = new ReentrantLock(true);
   
      public ConversationEntry(String id, List<String> stack, ConversationEntries parent)
      {
  @@ -58,13 +58,13 @@
         this.description = description;
      }
   
  -   public long getLastRequestTime() {
  +   public synchronized long getLastRequestTime() {
         return lastRequestTime;
      }
   
  -   void touch() {
  +   synchronized void touch() {
         parent.setDirty();
  -      this.lastRequestTime = System.currentTimeMillis();
  +      lastRequestTime = System.currentTimeMillis();
         lastDatetime = new Date();
      }
   
  @@ -110,7 +110,7 @@
         return viewId;
      }
   
  -   public Date getLastDatetime() {
  +   public synchronized Date getLastDatetime() {
         return lastDatetime;
      }
   
  @@ -176,11 +176,16 @@
         this.id = id;
      }
   
  -   public boolean lock()
  +   public boolean lockNoWait() //not synchronized!
  +   {
  +      return lock.tryLock();
  +   }
  +
  +   public boolean lock() //not synchronized!
      {
         try
         {
  -         return semaphore.tryAcquire( Manager.instance().getConcurrentRequestTimeout(), TimeUnit.MILLISECONDS );
  +         return lock.tryLock( Manager.instance().getConcurrentRequestTimeout(), TimeUnit.MILLISECONDS );
         }
         catch (InterruptedException ie)
         {
  @@ -188,9 +193,9 @@
         }
      }
      
  -   public void unlock()
  +   public void unlock() //not synchronized!
      {
  -      semaphore.release();
  +      lock.unlock();
      }
      
   }
  \ No newline at end of file
  
  
  
  1.96      +32 -5     jboss-seam/src/main/org/jboss/seam/core/Manager.java
  
  (In the diff below, changes in quantity of whitespace are not shown.)
  
  Index: Manager.java
  ===================================================================
  RCS file: /cvsroot/jboss/jboss-seam/src/main/org/jboss/seam/core/Manager.java,v
  retrieving revision 1.95
  retrieving revision 1.96
  diff -u -b -r1.95 -r1.96
  --- Manager.java	13 Oct 2006 01:27:31 -0000	1.95
  +++ Manager.java	13 Oct 2006 02:24:30 -0000	1.96
  @@ -41,7 +41,7 @@
    *
    * @author Gavin King
    * @author <a href="mailto:theute at jboss.org">Thomas Heute</a>
  - * @version $Revision: 1.95 $
  + * @version $Revision: 1.96 $
    */
   @Scope(ScopeType.EVENT)
   @Name("org.jboss.seam.core.manager")
  @@ -249,16 +249,41 @@
         List<ConversationEntry> entries = new ArrayList<ConversationEntry>( ConversationEntries.instance().getConversationEntries() );
         for (ConversationEntry conversationEntry: entries)
         {
  +         boolean locked = conversationEntry.lockNoWait(); //we had better not wait for it, or we would be waiting for ALL other requests
  +         try
  +         {
            long delta = currentTime - conversationEntry.getLastRequestTime();
            if ( delta > conversationEntry.getTimeout() )
            {
  -            if ( conversationEntry.lock() ) //no need to release it...
  +               if ( locked )
  +               { 
  +                  if ( log.isDebugEnabled() )
               {
                  log.debug("conversation timeout for conversation: " + conversationEntry.getId());
  +                  }
  +               }
  +               else
  +               {
  +                  //if we could not acquire the lock, someone has left a garbage lock lying around
  +                  //the reason garbage locks can exist is that we don't require a servlet filter to
  +                  //exist - but if we do use SeamExceptionFilter, it will clean up garbage and this
  +                  //case should never occur
  +                  
  +                  //NOTE: this is slightly broken - in theory there is a window where a new request 
  +                  //      could have come in and got the lock just before us but called touch() just 
  +                  //      after we check the timeout - but in practice this would be extremely rare, 
  +                  //      and that request will get an IllegalMonitorStateException when it tries to 
  +                  //      unlock() the CE
  +                  log.info("destroying conversation with garbage lock: " + conversationEntry.getId());
  +               }
                  ContextAdaptor session = ContextAdaptor.getSession(externalContext, true);
                  destroyConversation( conversationEntry.getId(), session );
               }
            }
  +         finally
  +         {
  +            if (locked) conversationEntry.unlock();
  +         }
         }
      }
   
  @@ -527,6 +552,8 @@
         if ( ce!=null && ce.lock() )
         {
   
  +         touchConversationStack();
  +
            //we found an id and obtained the lock, so restore the long-running conversation
            log.debug("Restoring conversation with id: " + storedConversationId);
            setLongRunningConversation(true);