Morning,

new patch is attached. It changes the name of the new service from
ObjectMutabilityAnalyzer to MutabilityAnalyzer and correctly keeps the
write lock locked when an attribute is written into the session and an
immutable object is read afterward.
Any more comments?

Jochen
From 78b331d9261b6ceb94dd0b93081becd02313c4cf Mon Sep 17 00:00:00 2001
From: Jochen Kemnade <jkemn...@apache.org>
Date: Wed, 28 May 2014 21:12:13 +0200
Subject: [PATCH] Downgrade the global session write lock to a read lock if an
 immutable object is read from the session

---
 .../internal/services/ClusteredSessionImpl.java    |   9 +-
 .../DefaultSessionPersistedObjectAnalyzer.java     |  16 ++-
 .../tapestry5/internal/services/SessionImpl.java   |  35 ++++++-
 .../tapestry5/internal/services/SessionLock.java   |   7 +-
 .../services/TapestrySessionFactoryImpl.java       |  33 +++++-
 .../apache/tapestry5/modules/TapestryModule.java   |  57 +++++++----
 .../tapestry5/services/MutabilityAnalyzer.java     |  44 ++++++++
 .../services/SessionPersistedObjectAnalyzer.java   |   3 +-
 .../internal/services/SessionImplTest.java         | 112 +++++++++++++++++++--
 .../tapestry5/services/MutabilityAnalyzerTest.java |  55 ++++++++++
 10 files changed, 326 insertions(+), 45 deletions(-)
 create mode 100644 tapestry-core/src/main/java/org/apache/tapestry5/services/MutabilityAnalyzer.java
 create mode 100644 tapestry-core/src/test/java/org/apache/tapestry5/services/MutabilityAnalyzerTest.java

diff --git a/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ClusteredSessionImpl.java b/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ClusteredSessionImpl.java
index 3dccb17..16892db 100644
--- a/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ClusteredSessionImpl.java
+++ b/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ClusteredSessionImpl.java
@@ -1,4 +1,4 @@
-//  Copyright 2011, 2013 The Apache Software Foundation
+//  Copyright 2011, 2013, 2014 The Apache Software Foundation
 //
 //  Licensed under the Apache License, Version 2.0 (the "License");
 //  you may not use this file except in compliance with the License.
@@ -15,11 +15,12 @@
 package org.apache.tapestry5.internal.services;
 
 import org.apache.tapestry5.ioc.internal.util.CollectionFactory;
-import org.apache.tapestry5.ioc.services.PerthreadManager;
+import org.apache.tapestry5.services.MutabilityAnalyzer;
 import org.apache.tapestry5.services.SessionPersistedObjectAnalyzer;
 
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpSession;
+
 import java.util.Map;
 
 /**
@@ -43,9 +44,9 @@ public class ClusteredSessionImpl extends SessionImpl
 
     public ClusteredSessionImpl(HttpServletRequest request,
             HttpSession session,
-            SessionLock lock, SessionPersistedObjectAnalyzer analyzer)
+            SessionLock lock, SessionPersistedObjectAnalyzer analyzer, MutabilityAnalyzer objectMutabilityAnalyzer)
     {
-        super(request, session, lock);
+        super(request, session, lock, objectMutabilityAnalyzer);
 
         this.analyzer = analyzer;
     }
diff --git a/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/DefaultSessionPersistedObjectAnalyzer.java b/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/DefaultSessionPersistedObjectAnalyzer.java
index 280aa12..751d0cb 100644
--- a/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/DefaultSessionPersistedObjectAnalyzer.java
+++ b/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/DefaultSessionPersistedObjectAnalyzer.java
@@ -1,4 +1,4 @@
-// Copyright 2008, 2011 The Apache Software Foundation
+// Copyright 2008, 2011, 2014 The Apache Software Foundation
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -14,7 +14,7 @@
 
 package org.apache.tapestry5.internal.services;
 
-import org.apache.tapestry5.annotations.ImmutableSessionPersistedObject;
+import org.apache.tapestry5.services.MutabilityAnalyzer;
 import org.apache.tapestry5.services.SessionPersistedObjectAnalyzer;
 
 /**
@@ -24,8 +24,16 @@ import org.apache.tapestry5.services.SessionPersistedObjectAnalyzer;
  */
 public class DefaultSessionPersistedObjectAnalyzer implements SessionPersistedObjectAnalyzer<Object>
 {
+    private final MutabilityAnalyzer objectMutabilityAnalyzer;
+
+    public DefaultSessionPersistedObjectAnalyzer(
+            MutabilityAnalyzer objectMutabilityAnalyzer) {
+        this.objectMutabilityAnalyzer = objectMutabilityAnalyzer;
+    }
+
     /**
-     * An object is dirty <em>unless</em> it has the {@link org.apache.tapestry5.annotations.ImmutableSessionPersistedObject}
+     * An object is dirty <em>unless</em> {@link org.apache.tapestry5.services.MutabilityAnalyzer}
+     * says differently, e.g. if its class has the {@link org.apache.tapestry5.annotations.ImmutableSessionPersistedObject}
      * annotation.
      *
      * @param sessionPersistedObject to analyze
@@ -33,7 +41,7 @@ public class DefaultSessionPersistedObjectAnalyzer implements SessionPersistedOb
      */
     public boolean checkAndResetDirtyState(Object sessionPersistedObject)
     {
-        boolean immutable = sessionPersistedObject.getClass().getAnnotation(ImmutableSessionPersistedObject.class) != null;
+        boolean immutable = objectMutabilityAnalyzer.isImmutable(sessionPersistedObject);
 
         // Immutable objects are always clean, others are assumed dirty.
         // Go implement OptimizedSessionPersistedObject if you don't like it.
diff --git a/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/SessionImpl.java b/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/SessionImpl.java
index 7829285..01ff4b2 100644
--- a/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/SessionImpl.java
+++ b/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/SessionImpl.java
@@ -1,4 +1,4 @@
-// Copyright 2006-2013 The Apache Software Foundation
+// Copyright 2006-2014 The Apache Software Foundation
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -16,11 +16,12 @@ package org.apache.tapestry5.internal.services;
 
 import org.apache.tapestry5.ioc.internal.util.CollectionFactory;
 import org.apache.tapestry5.ioc.internal.util.InternalUtils;
-import org.apache.tapestry5.ioc.services.PerthreadManager;
+import org.apache.tapestry5.services.MutabilityAnalyzer;
 import org.apache.tapestry5.services.Session;
 
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpSession;
+
 import java.util.Collections;
 import java.util.Enumeration;
 import java.util.List;
@@ -36,20 +37,42 @@ public class SessionImpl implements Session
 
     private boolean invalidated = false;
 
+    private boolean needToKeepWriteLock = false;
+
     private final SessionLock lock;
 
-    public SessionImpl(HttpServletRequest request, HttpSession session, SessionLock lock)
+    private final MutabilityAnalyzer objectMutabilityAnalyzer;
+
+
+
+    public SessionImpl(HttpServletRequest request, HttpSession session, SessionLock lock, MutabilityAnalyzer objectMutabilityAnalyzer)
     {
         this.request = request;
         this.session = session;
         this.lock = lock;
+        this.objectMutabilityAnalyzer = objectMutabilityAnalyzer;
     }
 
     public Object getAttribute(String name)
     {
+        // We acquire the write lock as a precaution because the object that is to be read might be mutable
         lock.acquireWriteLock();
 
-        return session.getAttribute(name);
+        Object attributeValue = session.getAttribute(name);
+
+        // If no mutable object was read from the session before and no attribute was written,
+        // we may be able to release the write lock
+        if (!needToKeepWriteLock)
+        {
+            if (objectMutabilityAnalyzer.isImmutable(attributeValue))
+            {
+                lock.downgradeWriteLockToReadLock();
+            } else
+            {
+                needToKeepWriteLock = true;
+            }
+        }
+        return attributeValue;
     }
 
     public List<String> getAttributeNames()
@@ -63,6 +86,10 @@ public class SessionImpl implements Session
     {
         lock.acquireWriteLock();
 
+        // If we write an attribute, we will definitely have to keep the write lock locked,
+        // regardless of whether every attribute we *read* is immutable
+        needToKeepWriteLock = true;
+
         session.setAttribute(name, value);
     }
 
diff --git a/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/SessionLock.java b/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/SessionLock.java
index d234bd4..cea240f 100644
--- a/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/SessionLock.java
+++ b/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/SessionLock.java
@@ -1,4 +1,4 @@
-//  Copyright 2013 The Apache Software Foundation
+//  Copyright 2013, 2014 The Apache Software Foundation
 //
 //  Licensed under the Apache License, Version 2.0 (the "License");
 //  you may not use this file except in compliance with the License.
@@ -33,4 +33,9 @@ public interface SessionLock
      * this may block for a while. Does nothing if the write lock is already held by this thread.
      */
     void acquireWriteLock();
+
+    /**
+     * Releases the exclusive write lock and acquires the read lock instead.
+     */
+    void downgradeWriteLockToReadLock();
 }
diff --git a/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/TapestrySessionFactoryImpl.java b/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/TapestrySessionFactoryImpl.java
index bc221a6..3ca663b 100644
--- a/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/TapestrySessionFactoryImpl.java
+++ b/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/TapestrySessionFactoryImpl.java
@@ -1,4 +1,4 @@
-//  Copyright 2011, 2013 The Apache Software Foundation
+//  Copyright 2011, 2013, 2014 The Apache Software Foundation
 //
 //  Licensed under the Apache License, Version 2.0 (the "License");
 //  you may not use this file except in compliance with the License.
@@ -17,11 +17,13 @@ package org.apache.tapestry5.internal.services;
 import org.apache.tapestry5.SymbolConstants;
 import org.apache.tapestry5.ioc.annotations.Symbol;
 import org.apache.tapestry5.ioc.services.PerthreadManager;
+import org.apache.tapestry5.services.MutabilityAnalyzer;
 import org.apache.tapestry5.services.Session;
 import org.apache.tapestry5.services.SessionPersistedObjectAnalyzer;
 
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpSession;
+
 import java.util.Map;
 import java.util.WeakHashMap;
 import java.util.concurrent.locks.Lock;
@@ -34,6 +36,8 @@ public class TapestrySessionFactoryImpl implements TapestrySessionFactory
 
     private final SessionPersistedObjectAnalyzer analyzer;
 
+    private final MutabilityAnalyzer objectMutabilityAnalyzer;
+
     private final HttpServletRequest request;
 
     private final PerthreadManager perthreadManager;
@@ -53,6 +57,10 @@ public class TapestrySessionFactoryImpl implements TapestrySessionFactory
         public void acquireWriteLock()
         {
         }
+
+        public void downgradeWriteLockToReadLock()
+        {
+        }
     };
 
     private class SessionLockImpl implements SessionLock
@@ -112,17 +120,31 @@ public class TapestrySessionFactoryImpl implements TapestrySessionFactory
             {
                 public void run()
                 {
-                    // This is the only way a write lock is unlocked, so no check is needed.
-                    lock.writeLock().unlock();
+                    // The write lock may have been released, if downgraded to a read lock.
+                    if (isWriteLocked())
+                    {
+                        lock.writeLock().unlock();
+                    }
                 }
             });
         }
+
+        @Override
+        public void downgradeWriteLockToReadLock()
+        {
+            lock.writeLock().unlock();
+
+            // During this window, no lock is held, and the next call may block.
+
+            acquireReadLock();
+        }
     }
 
     public TapestrySessionFactoryImpl(
             @Symbol(SymbolConstants.CLUSTERED_SESSIONS)
             boolean clustered,
             SessionPersistedObjectAnalyzer analyzer,
+            MutabilityAnalyzer objectMutabilityAnalyzer,
             HttpServletRequest request,
             PerthreadManager perthreadManager,
             @Symbol(SymbolConstants.SESSION_LOCKING_ENABLED)
@@ -130,6 +152,7 @@ public class TapestrySessionFactoryImpl implements TapestrySessionFactory
     {
         this.clustered = clustered;
         this.analyzer = analyzer;
+        this.objectMutabilityAnalyzer = objectMutabilityAnalyzer;
         this.request = request;
         this.perthreadManager = perthreadManager;
         this.sessionLockingEnabled = sessionLockingEnabled;
@@ -148,10 +171,10 @@ public class TapestrySessionFactoryImpl implements TapestrySessionFactory
 
         if (clustered)
         {
-            return new ClusteredSessionImpl(request, httpSession, lock, analyzer);
+            return new ClusteredSessionImpl(request, httpSession, lock, analyzer, objectMutabilityAnalyzer);
         }
 
-        return new SessionImpl(request, httpSession, lock);
+        return new SessionImpl(request, httpSession, lock, objectMutabilityAnalyzer);
     }
 
     private SessionLock lockForSession(HttpSession session)
diff --git a/tapestry-core/src/main/java/org/apache/tapestry5/modules/TapestryModule.java b/tapestry-core/src/main/java/org/apache/tapestry5/modules/TapestryModule.java
index e520b09..fd5f98d 100644
--- a/tapestry-core/src/main/java/org/apache/tapestry5/modules/TapestryModule.java
+++ b/tapestry-core/src/main/java/org/apache/tapestry5/modules/TapestryModule.java
@@ -94,6 +94,7 @@ import org.slf4j.Logger;
 import javax.servlet.ServletContext;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
+
 import java.io.IOException;
 import java.lang.annotation.Annotation;
 import java.math.BigDecimal;
@@ -2323,29 +2324,16 @@ public final class TapestryModule
     }
 
     /**
-     * Identifies String, Number and Boolean as immutable objects, a catch-all
-     * handler for Object (that understands
-     * the {@link org.apache.tapestry5.annotations.ImmutableSessionPersistedObject} annotation),
-     * and a handler for {@link org.apache.tapestry5.OptimizedSessionPersistedObject}.
+     * Uses MutabilityAnalyzer to determine if an object is immutable, and adds
+     * a handler for {@link org.apache.tapestry5.OptimizedSessionPersistedObject}.
      *
      * @since 5.1.0.0
      */
     public static void contributeSessionPersistedObjectAnalyzer(
-            MappedConfiguration<Class, SessionPersistedObjectAnalyzer> configuration)
+            MappedConfiguration<Class, SessionPersistedObjectAnalyzer> configuration,
+            MutabilityAnalyzer objectMutabilityAnalyzer)
     {
-        configuration.add(Object.class, new DefaultSessionPersistedObjectAnalyzer());
-
-        SessionPersistedObjectAnalyzer<Object> immutable = new SessionPersistedObjectAnalyzer<Object>()
-        {
-            public boolean checkAndResetDirtyState(Object sessionPersistedObject)
-            {
-                return false;
-            }
-        };
-
-        configuration.add(String.class, immutable);
-        configuration.add(Number.class, immutable);
-        configuration.add(Boolean.class, immutable);
+        configuration.add(Object.class, new DefaultSessionPersistedObjectAnalyzer(objectMutabilityAnalyzer));
 
         configuration.add(OptimizedSessionPersistedObject.class, new OptimizedSessionPersistedObjectAnalyzer());
     }
@@ -2654,4 +2642,37 @@ public final class TapestryModule
     {
         return strategyBuilder.build(ValueLabelProvider.class, configuration);
     }
+
+    /**
+     * @since 5.4
+     */
+    @Marker(Primary.class)
+    public MutabilityAnalyzer buildMutabilityAnalyzer(List<MutabilityAnalyzer> configuration)
+    {
+        return chainBuilder.build(MutabilityAnalyzer.class, configuration);
+    }
+
+    @Contribute(MutabilityAnalyzer.class)
+    public static void addCodeMutabilityAnalyzers(OrderedConfiguration<MutabilityAnalyzer> configuration){
+        configuration.add("null", new MutabilityAnalyzer(){
+            public boolean isImmutable(Object object) {
+                return object == null;
+            }
+        }, OrderConstraintBuilder.beforeAll().build());
+
+        configuration.add("ImmutableClass", new MutabilityAnalyzer(){
+            private final Set<Class> immutableClasses = Collections.unmodifiableSet(
+                    CollectionFactory.<Class, Class>newSet(String.class, Character.class, Byte.class,
+                        Integer.class, Long.class, Short.class, Double.class, Boolean.class));
+            public boolean isImmutable(Object object) {
+                return immutableClasses.contains(object.getClass());
+            }
+        });
+
+        configuration.add("ImmutableSessionPersistedObject", new MutabilityAnalyzer(){
+            public boolean isImmutable(Object object) {
+                return object.getClass().getAnnotation(ImmutableSessionPersistedObject.class) != null;
+            }
+        });
+    }
 }
diff --git a/tapestry-core/src/main/java/org/apache/tapestry5/services/MutabilityAnalyzer.java b/tapestry-core/src/main/java/org/apache/tapestry5/services/MutabilityAnalyzer.java
new file mode 100644
index 0000000..020171a
--- /dev/null
+++ b/tapestry-core/src/main/java/org/apache/tapestry5/services/MutabilityAnalyzer.java
@@ -0,0 +1,44 @@
+// Copyright 2014 The Apache Software Foundation
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package org.apache.tapestry5.services;
+
+import org.apache.tapestry5.ioc.annotations.UsesOrderedConfiguration;
+import org.apache.tapestry5.ioc.services.ChainBuilder;
+
+/**
+ * Analyzes an object for mutability. If an object is immutable, some optimizations can be made
+ * e.g. when storing it as a HTTP session attribute.
+ * <p/>
+ * The service implementation is a {@linkplain ChainBuilder
+ * chain of command}. The service may be
+ * contributed to using the {@link org.apache.tapestry5.ioc.annotations.Primary} marker annotation.
+ *
+ * @see org.apache.tapestry5.annotations.ImmutableSessionPersistedObject
+ * @see org.apache.tapestry5.OptimizedSessionPersistedObject
+ * @see org.apache.tapestry5.internal.services.SessionLock
+ * @since 5.4
+ */
+@UsesOrderedConfiguration(MutabilityAnalyzer.class)
+public interface MutabilityAnalyzer
+{
+    /**
+     * Check if an object is immutable
+     * @param object the object to analyze
+     * @return {@code true} if the object is immutable
+     * @since 5.4
+     */
+    boolean isImmutable(Object object);
+
+}
diff --git a/tapestry-core/src/main/java/org/apache/tapestry5/services/SessionPersistedObjectAnalyzer.java b/tapestry-core/src/main/java/org/apache/tapestry5/services/SessionPersistedObjectAnalyzer.java
index 950fbeb..99ffef4 100644
--- a/tapestry-core/src/main/java/org/apache/tapestry5/services/SessionPersistedObjectAnalyzer.java
+++ b/tapestry-core/src/main/java/org/apache/tapestry5/services/SessionPersistedObjectAnalyzer.java
@@ -1,4 +1,4 @@
-// Copyright 2008, 2011 The Apache Software Foundation
+// Copyright 2008, 2011, 2014 The Apache Software Foundation
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -29,6 +29,7 @@ import org.apache.tapestry5.ioc.annotations.UsesMappedConfiguration;
  *
  * @see org.apache.tapestry5.annotations.ImmutableSessionPersistedObject
  * @see org.apache.tapestry5.OptimizedSessionPersistedObject
+ * @see org.apache.tapestry5.services.MutabilityAnalyzer
  * @since 5.1.0.0
  */
 @UsesMappedConfiguration(key = Class.class, value = SessionPersistedObjectAnalyzer.class)
diff --git a/tapestry-core/src/test/java/org/apache/tapestry5/internal/services/SessionImplTest.java b/tapestry-core/src/test/java/org/apache/tapestry5/internal/services/SessionImplTest.java
index 4f1aebf..41ceadd 100644
--- a/tapestry-core/src/test/java/org/apache/tapestry5/internal/services/SessionImplTest.java
+++ b/tapestry-core/src/test/java/org/apache/tapestry5/internal/services/SessionImplTest.java
@@ -1,4 +1,4 @@
-// Copyright 2006-2013 The Apache Software Foundation
+// Copyright 2006-2014 The Apache Software Foundation
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -15,12 +15,14 @@
 package org.apache.tapestry5.internal.services;
 
 import org.apache.tapestry5.internal.test.InternalBaseTestCase;
+import org.apache.tapestry5.services.MutabilityAnalyzer;
 import org.apache.tapestry5.services.Session;
 import org.apache.tapestry5.services.SessionPersistedObjectAnalyzer;
 import org.testng.annotations.Test;
 
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpSession;
+
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.Enumeration;
@@ -33,19 +35,25 @@ public class SessionImplTest extends InternalBaseTestCase
         return newMock(SessionLock.class);
     }
 
+    private MutabilityAnalyzer mockMutabilityAnalyzer()
+    {
+        return newMock(MutabilityAnalyzer.class);
+    }
+
     @Test
     public void get_attribute_names()
     {
         Enumeration e = Collections.enumeration(Arrays.asList("fred", "barney"));
         HttpSession hs = mockHttpSession();
         SessionLock lock = mockLock();
+        MutabilityAnalyzer mutabilityAnalyzer = mockMutabilityAnalyzer();
 
         lock.acquireReadLock();
         expect(hs.getAttributeNames()).andReturn(e);
 
         replay();
 
-        Session session = new SessionImpl(null, hs, lock);
+        Session session = new SessionImpl(null, hs, lock, mutabilityAnalyzer);
 
         assertEquals(session.getAttributeNames(), Arrays.asList("barney", "fred"));
 
@@ -58,6 +66,7 @@ public class SessionImplTest extends InternalBaseTestCase
         Enumeration e = Collections.enumeration(Arrays.asList("fred", "barney", "fanny"));
         HttpSession hs = mockHttpSession();
         SessionLock lock = mockLock();
+        MutabilityAnalyzer mutabilityAnalyzer = mockMutabilityAnalyzer();
 
         lock.acquireReadLock();
 
@@ -65,7 +74,7 @@ public class SessionImplTest extends InternalBaseTestCase
 
         replay();
 
-        Session session = new SessionImpl(null, hs, lock);
+        Session session = new SessionImpl(null, hs, lock, mutabilityAnalyzer);
 
         assertEquals(session.getAttributeNames("f"), Arrays.asList("fanny", "fred"));
 
@@ -77,12 +86,13 @@ public class SessionImplTest extends InternalBaseTestCase
     {
         HttpSession hs = mockHttpSession();
         SessionLock lock = mockLock();
+        MutabilityAnalyzer mutabilityAnalyzer = mockMutabilityAnalyzer();
 
         hs.invalidate();
 
         replay();
 
-        Session session = new SessionImpl(null, hs, lock);
+        Session session = new SessionImpl(null, hs, lock, mutabilityAnalyzer);
 
         session.invalidate();
 
@@ -95,12 +105,13 @@ public class SessionImplTest extends InternalBaseTestCase
         HttpSession hs = mockHttpSession();
         HttpServletRequest hsr = mockHttpServletRequest();
         SessionLock lock = mockLock();
+        MutabilityAnalyzer mutabilityAnalyzer = mockMutabilityAnalyzer();
 
         train_getSession(hsr, false, hs);
 
         replay();
 
-        Session session = new SessionImpl(hsr, hs, lock);
+        Session session = new SessionImpl(hsr, hs, lock, mutabilityAnalyzer);
 
         assertFalse(session.isInvalidated());
 
@@ -126,13 +137,14 @@ public class SessionImplTest extends InternalBaseTestCase
     public void set_max_inactive()
     {
         HttpSession hs = mockHttpSession();
+        MutabilityAnalyzer mutabilityAnalyzer = mockMutabilityAnalyzer();
         int seconds = 999;
 
         hs.setMaxInactiveInterval(seconds);
 
         replay();
 
-        Session session = new SessionImpl(null, hs, null);
+        Session session = new SessionImpl(null, hs, null, mutabilityAnalyzer);
 
         session.setMaxInactiveInterval(seconds);
 
@@ -143,6 +155,7 @@ public class SessionImplTest extends InternalBaseTestCase
     public void get_max_inactive()
     {
         HttpSession hs = mockHttpSession();
+        MutabilityAnalyzer mutabilityAnalyzer = mockMutabilityAnalyzer();
 
         int seconds = 999;
 
@@ -150,7 +163,7 @@ public class SessionImplTest extends InternalBaseTestCase
 
         replay();
 
-        Session session = new SessionImpl(null, hs, null);
+        Session session = new SessionImpl(null, hs, null, mutabilityAnalyzer);
 
         assertEquals(session.getMaxInactiveInterval(), seconds);
 
@@ -163,16 +176,18 @@ public class SessionImplTest extends InternalBaseTestCase
         HttpSession hs = mockHttpSession();
         HttpServletRequest hsr = mockHttpServletRequest();
         SessionPersistedObjectAnalyzer analyzer = newMock(SessionPersistedObjectAnalyzer.class);
+        MutabilityAnalyzer mutabilityAnalyzer = mockMutabilityAnalyzer();
         Object dirty = new Object();
         SessionLock lock = mockLock();
 
         lock.acquireWriteLock();
 
         train_getAttribute(hs, "dirty", dirty);
+        expect(mutabilityAnalyzer.isImmutable(dirty)).andReturn(false);
 
         replay();
 
-        Session session = new ClusteredSessionImpl(hsr, hs, lock, analyzer);
+        Session session = new ClusteredSessionImpl(hsr, hs, lock, analyzer, mutabilityAnalyzer);
 
         assertSame(session.getAttribute("dirty"), dirty);
 
@@ -192,4 +207,85 @@ public class SessionImplTest extends InternalBaseTestCase
 
         verify();
     }
+
+    @Test
+    public void reading_an_immutable_object_releases_the_write_lock()
+    {
+        HttpSession hs = mockHttpSession();
+        HttpServletRequest hsr = mockHttpServletRequest();
+        SessionPersistedObjectAnalyzer analyzer = newMock(SessionPersistedObjectAnalyzer.class);
+        MutabilityAnalyzer mutabilityAnalyzer = mockMutabilityAnalyzer();
+        Object immutable = new Object();
+        SessionLock lock = mockLock();
+
+        lock.acquireWriteLock();
+
+        train_getAttribute(hs, "immutable", immutable);
+        expect(mutabilityAnalyzer.isImmutable(immutable)).andReturn(true);
+        lock.downgradeWriteLockToReadLock();
+        replay();
+
+        Session session = new ClusteredSessionImpl(hsr, hs, lock, analyzer, mutabilityAnalyzer);
+
+        assertSame(session.getAttribute("immutable"), immutable);
+
+        verify();
+
+    }
+
+    @Test
+    public void reading_a_mutable_and_an_immutable_object_keeps_the_write_lock()
+    {
+        HttpSession hs = mockHttpSession();
+        HttpServletRequest hsr = mockHttpServletRequest();
+        SessionPersistedObjectAnalyzer analyzer = newMock(SessionPersistedObjectAnalyzer.class);
+        MutabilityAnalyzer mutabilityAnalyzer = mockMutabilityAnalyzer();
+        Object immutable = new Object();
+        Object mutable = new Object();
+        SessionLock lock = mockLock();
+
+        lock.acquireWriteLock();
+
+        train_getAttribute(hs, "mutable", mutable);
+        expect(mutabilityAnalyzer.isImmutable(mutable)).andReturn(false);
+
+        train_getAttribute(hs, "immutable", immutable);
+        lock.acquireWriteLock();
+        replay();
+
+        Session session = new ClusteredSessionImpl(hsr, hs, lock, analyzer, mutabilityAnalyzer);
+
+        assertSame(session.getAttribute("mutable"), mutable);
+        assertSame(session.getAttribute("immutable"), immutable);
+
+        verify();
+
+    }
+
+    @Test
+    public void writing_an_attribute_and_reading_an_immutable_one_keeps_the_write_lock()
+    {
+        HttpSession hs = mockHttpSession();
+        HttpServletRequest hsr = mockHttpServletRequest();
+        SessionPersistedObjectAnalyzer analyzer = newMock(SessionPersistedObjectAnalyzer.class);
+        MutabilityAnalyzer mutabilityAnalyzer = mockMutabilityAnalyzer();
+        Object o = new Object();
+        Object immutable = new Object();
+
+        SessionLock lock = mockLock();
+
+        lock.acquireWriteLock();
+        hs.setAttribute("attribute", o);
+        train_getAttribute(hs, "immutable", immutable);
+        lock.acquireWriteLock();
+
+        replay();
+
+        Session session = new ClusteredSessionImpl(hsr, hs, lock, analyzer, mutabilityAnalyzer);
+        session.setAttribute("attribute", o);
+        assertSame(session.getAttribute("immutable"), immutable);
+
+        verify();
+
+    }
 }
diff --git a/tapestry-core/src/test/java/org/apache/tapestry5/services/MutabilityAnalyzerTest.java b/tapestry-core/src/test/java/org/apache/tapestry5/services/MutabilityAnalyzerTest.java
new file mode 100644
index 0000000..6c8c13d
--- /dev/null
+++ b/tapestry-core/src/test/java/org/apache/tapestry5/services/MutabilityAnalyzerTest.java
@@ -0,0 +1,55 @@
+// Copyright 2014 The Apache Software Foundation
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package org.apache.tapestry5.services;
+
+import java.util.HashMap;
+
+import org.apache.tapestry5.annotations.ImmutableSessionPersistedObject;
+import org.apache.tapestry5.internal.test.InternalBaseTestCase;
+import org.testng.annotations.Test;
+
+public class MutabilityAnalyzerTest extends InternalBaseTestCase
+{
+    @Test
+    public void mutable_objects()
+    {
+
+        MutabilityAnalyzer analyzer = getService(MutabilityAnalyzer.class);
+
+        assertFalse(analyzer.isImmutable(new HashMap()));
+        assertFalse(analyzer.isImmutable(new String[]{"foo"}));
+
+    }
+
+    @Test
+    public void immutable_objects()
+    {
+
+        MutabilityAnalyzer analyzer = getService(MutabilityAnalyzer.class);
+
+        assertTrue(analyzer.isImmutable(true));
+        assertTrue(analyzer.isImmutable(Boolean.TRUE));
+        assertTrue(analyzer.isImmutable(null));
+        assertTrue(analyzer.isImmutable("Hello World"));
+        assertTrue(analyzer.isImmutable(42));
+        assertTrue(analyzer.isImmutable(new ImmutableObject()));
+
+    }
+
+    @ImmutableSessionPersistedObject
+    private class ImmutableObject {
+
+    }
+}
-- 
1.9.1

Attachment: signature.asc
Description: OpenPGP digital signature

Reply via email to