Tapestry 5
  1. Tapestry 5
  2. TAP5-2049

Tapestry should provide locking semantics for attributes stored in the session, to prevent multiple simultaneous requests (due to Ajax) from conflicting

    Details

      Description

      Currently, multiple threads may be processing requests, due to Ajax. If they all access state stored in the HttpSession, the rsult can be invalid.

      Tapestry should maintain a lock object inside the session, and automatically acquire a read lock the first time a request thread reads values from the session, and upgrade to a write lock when writing values to the session.

      The lock can be released at the end of the request.

      A reentrant read/write lock may be insufficient, as values inside the session are mutable; it is possible that the lock should be exclusive.

        Issue Links

          Activity

          Hide
          Hudson added a comment -

          Integrated in tapestry-trunk-freestyle #1010 (See https://builds.apache.org/job/tapestry-trunk-freestyle/1010/)
          TAP5-2049: Use a lock when reading/updating HttpSession attributes (Revision 680ffc3a3b8216dc73a4de61c6c332cff9836147)
          TAP5-2049: Use a lock when reading/updating HttpSession attributes (Revision b7b44b8fa4b99a9d04953b2a7e9662714add2891)
          TAP5-2049: Use a lock when reading/updating HttpSession attributes (Revision 1da50f173f1985c39d5ab82f1f22d516857ca748)

          Result = ABORTED
          hlship :
          Files :

          • tapestry-core/src/main/java/org/apache/tapestry5/internal/services/SessionImpl.java
          • tapestry-core/src/main/java/org/apache/tapestry5/internal/services/SessionLock.java
          • tapestry-core/src/main/java/org/apache/tapestry5/internal/services/TapestrySessionFactoryImpl.java
          • tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ClusteredSessionImpl.java
          • tapestry-core/src/test/java/org/apache/tapestry5/internal/services/SessionImplTest.java

          hlship :
          Files :

          • tapestry-core/src/main/java/org/apache/tapestry5/services/TapestryModule.java
          • tapestry-core/src/main/java/org/apache/tapestry5/SymbolConstants.java
          • tapestry-core/src/main/java/org/apache/tapestry5/internal/services/TapestrySessionFactoryImpl.java

          hlship :
          Files :

          • 54_RELEASE_NOTES.txt
          Show
          Hudson added a comment - Integrated in tapestry-trunk-freestyle #1010 (See https://builds.apache.org/job/tapestry-trunk-freestyle/1010/ ) TAP5-2049 : Use a lock when reading/updating HttpSession attributes (Revision 680ffc3a3b8216dc73a4de61c6c332cff9836147) TAP5-2049 : Use a lock when reading/updating HttpSession attributes (Revision b7b44b8fa4b99a9d04953b2a7e9662714add2891) TAP5-2049 : Use a lock when reading/updating HttpSession attributes (Revision 1da50f173f1985c39d5ab82f1f22d516857ca748) Result = ABORTED hlship : Files : tapestry-core/src/main/java/org/apache/tapestry5/internal/services/SessionImpl.java tapestry-core/src/main/java/org/apache/tapestry5/internal/services/SessionLock.java tapestry-core/src/main/java/org/apache/tapestry5/internal/services/TapestrySessionFactoryImpl.java tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ClusteredSessionImpl.java tapestry-core/src/test/java/org/apache/tapestry5/internal/services/SessionImplTest.java hlship : Files : tapestry-core/src/main/java/org/apache/tapestry5/services/TapestryModule.java tapestry-core/src/main/java/org/apache/tapestry5/SymbolConstants.java tapestry-core/src/main/java/org/apache/tapestry5/internal/services/TapestrySessionFactoryImpl.java hlship : Files : 54_RELEASE_NOTES.txt
          Hide
          Howard M. Lewis Ship added a comment -

          Please take the discussion to the Tapestry developer mailing list.

          Show
          Howard M. Lewis Ship added a comment - Please take the discussion to the Tapestry developer mailing list.
          Hide
          Denis Stepanov added a comment -

          Obviously, exclusive session access is a bad idea:

          • We have many concurrent ajax request, this change means major performance issue for us!
          • This will not work in a clustered environment, the clustered session class shouldn't inherit the locks functionality.
          • Tapestry should not do this by default, any kind of synchronization between the requests is bad idea and should be avoided at any cost.

          Please do not force every Tapestry user to have this, if you want it in your apps just advice the session factory and return exclusive session delegate.

          Show
          Denis Stepanov added a comment - Obviously, exclusive session access is a bad idea: We have many concurrent ajax request, this change means major performance issue for us! This will not work in a clustered environment, the clustered session class shouldn't inherit the locks functionality. Tapestry should not do this by default, any kind of synchronization between the requests is bad idea and should be avoided at any cost. Please do not force every Tapestry user to have this, if you want it in your apps just advice the session factory and return exclusive session delegate.
          Hide
          Howard M. Lewis Ship added a comment -

          Obviously, we only can handle locking within a single JVM. ReentrantReadWriteLock is serializable, so it will transfer between servers in the cluster just fine.

          Thread safety is paramount for me; if a single browser is firing many Ajax requests simultaneously, they may be handled in sequential order (when there is server-side state). I am already considering a tweak where querying the names of attributes only requires a shared read lock, and then an upgrade to exclusive write lock only occurs when reading or updating attributes.

          Brian Goetz has covered this issue:
          http://www.ibm.com/developerworks/library/j-jtp09238/index.html

          Show
          Howard M. Lewis Ship added a comment - Obviously, we only can handle locking within a single JVM. ReentrantReadWriteLock is serializable, so it will transfer between servers in the cluster just fine. Thread safety is paramount for me; if a single browser is firing many Ajax requests simultaneously, they may be handled in sequential order (when there is server-side state). I am already considering a tweak where querying the names of attributes only requires a shared read lock, and then an upgrade to exclusive write lock only occurs when reading or updating attributes. Brian Goetz has covered this issue: http://www.ibm.com/developerworks/library/j-jtp09238/index.html
          Hide
          Denis Stepanov added a comment -

          Why would you do this?

          This should be handled by some service not by the session. One long running could lock the session, it could mean performance issue.

          Also, storing ReentrantLock in the session is not a good idea, what if it is clustered?

          How would you prevent multiple simultaneous requests in the cluster environment? It doesn't makes sense at all.

          Show
          Denis Stepanov added a comment - Why would you do this? This should be handled by some service not by the session. One long running could lock the session, it could mean performance issue. Also, storing ReentrantLock in the session is not a good idea, what if it is clustered? How would you prevent multiple simultaneous requests in the cluster environment? It doesn't makes sense at all.
          Hide
          Hudson added a comment -

          Integrated in tapestry-trunk-freestyle #1001 (See https://builds.apache.org/job/tapestry-trunk-freestyle/1001/)
          TAP5-2049: Use a lock when reading/updating HttpSession attributes (Revision bebe2d1417fcfa0b32bd8aa8abd246b3e0c1504c)

          Result = SUCCESS
          hlship :
          Files :

          • tapestry-core/src/test/java/org/apache/tapestry5/internal/services/SessionImplTest.java
          • tapestry-core/src/main/java/org/apache/tapestry5/internal/services/RequestImpl.java
          • tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ClusteredSessionImpl.java
          • tapestry-core/src/main/java/org/apache/tapestry5/internal/services/SessionImpl.java
          • tapestry-core/src/main/java/org/apache/tapestry5/internal/services/TapestrySessionFactoryImpl.java
          • tapestry-core/src/test/java/org/apache/tapestry5/internal/services/RequestImplTest.java
          Show
          Hudson added a comment - Integrated in tapestry-trunk-freestyle #1001 (See https://builds.apache.org/job/tapestry-trunk-freestyle/1001/ ) TAP5-2049 : Use a lock when reading/updating HttpSession attributes (Revision bebe2d1417fcfa0b32bd8aa8abd246b3e0c1504c) Result = SUCCESS hlship : Files : tapestry-core/src/test/java/org/apache/tapestry5/internal/services/SessionImplTest.java tapestry-core/src/main/java/org/apache/tapestry5/internal/services/RequestImpl.java tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ClusteredSessionImpl.java tapestry-core/src/main/java/org/apache/tapestry5/internal/services/SessionImpl.java tapestry-core/src/main/java/org/apache/tapestry5/internal/services/TapestrySessionFactoryImpl.java tapestry-core/src/test/java/org/apache/tapestry5/internal/services/RequestImplTest.java

            People

            • Assignee:
              Howard M. Lewis Ship
              Reporter:
              Howard M. Lewis Ship
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development