I find this change has too extensive imiplications as to default it to true.
Also, I find it much to coarse-grained to either enable it for the whole application or for nothing. An annotation on event handlers that require synchronized access to the session is the way to go IMO. Uli On 17.01.2013 18:55, hls...@apache.org wrote: > Updated Branches: > refs/heads/master fb3fca8d0 -> b7b44b8fa > > > TAP5-2049: Use a lock when reading/updating HttpSession attributes > - Add a symbol to deactivate the session locking logic > > > Project: http://git-wip-us.apache.org/repos/asf/tapestry-5/repo > Commit: http://git-wip-us.apache.org/repos/asf/tapestry-5/commit/b7b44b8f > Tree: http://git-wip-us.apache.org/repos/asf/tapestry-5/tree/b7b44b8f > Diff: http://git-wip-us.apache.org/repos/asf/tapestry-5/diff/b7b44b8f > > Branch: refs/heads/master > Commit: b7b44b8fa4b99a9d04953b2a7e9662714add2891 > Parents: 680ffc3 > Author: Howard M. Lewis Ship <hls...@apache.org> > Authored: Thu Jan 17 17:54:59 2013 +0000 > Committer: Howard M. Lewis Ship <hls...@apache.org> > Committed: Thu Jan 17 17:54:59 2013 +0000 > > ---------------------------------------------------------------------- > .../java/org/apache/tapestry5/SymbolConstants.java | 15 +++++++++- > .../services/TapestrySessionFactoryImpl.java | 23 ++++++++++++++- > .../apache/tapestry5/services/TapestryModule.java | 2 + > 3 files changed, 38 insertions(+), 2 deletions(-) > ---------------------------------------------------------------------- > > > http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/b7b44b8f/tapestry-core/src/main/java/org/apache/tapestry5/SymbolConstants.java > ---------------------------------------------------------------------- > diff --git > a/tapestry-core/src/main/java/org/apache/tapestry5/SymbolConstants.java > b/tapestry-core/src/main/java/org/apache/tapestry5/SymbolConstants.java > index 4ee6fc5..44f4ae8 100644 > --- a/tapestry-core/src/main/java/org/apache/tapestry5/SymbolConstants.java > +++ b/tapestry-core/src/main/java/org/apache/tapestry5/SymbolConstants.java > @@ -1,4 +1,4 @@ > -// Copyright 2008-2013The Apache Software Foundation > +// Copyright 2008-2013 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. > @@ -382,4 +382,17 @@ public class SymbolConstants > * @since 5.4 > */ > public static final String JAVASCRIPT_INFRASTRUCTURE_PROVIDER = > "tapestry.javascript-infrastructure-provider"; > + > + /** > + * If true (the default), then Tapestry will apply locking semantics > around access to the {@link javax.servlet.http.HttpSession}. > + * Reading attribute names occurs with a shared read lock; getting or > setting an attribute upgrades to an exclusive write lock. > + * This can tend to serialize threads when a number of simultaneous > (Ajax) requests from the client arrive ... however, > + * many implementations of HttpSession are not thread safe, and often > mutable objects are stored in the session and shared > + * between threads. Leaving this on the default will yield a more robust > application; setting it to false may speed > + * up processing for more Ajax intensive applications (but care should > then be given to ensuring that objects shared inside > + * the session are themeselves immutable or thread-safe). > + * > + * @since 5.4 > + */ > + public static final String SESSION_LOCKING_ENABLED = > "tapestry.session-locking-enabled"; > } > > http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/b7b44b8f/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/TapestrySessionFactoryImpl.java > ---------------------------------------------------------------------- > 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 3db2d28..bc221a6 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 > @@ -38,10 +38,23 @@ public class TapestrySessionFactoryImpl implements > TapestrySessionFactory > > private final PerthreadManager perthreadManager; > > + private final boolean sessionLockingEnabled; > + > private final Lock mapLock = new ReentrantLock(); > > private final Map<HttpSession, SessionLock> sessionToLock = new > WeakHashMap<HttpSession, SessionLock>(); > > + private final SessionLock NO_OP_LOCK = new SessionLock() > + { > + public void acquireReadLock() > + { > + } > + > + public void acquireWriteLock() > + { > + } > + }; > + > private class SessionLockImpl implements SessionLock > { > > @@ -111,12 +124,15 @@ public class TapestrySessionFactoryImpl implements > TapestrySessionFactory > boolean clustered, > SessionPersistedObjectAnalyzer analyzer, > HttpServletRequest request, > - PerthreadManager perthreadManager) > + PerthreadManager perthreadManager, > + @Symbol(SymbolConstants.SESSION_LOCKING_ENABLED) > + boolean sessionLockingEnabled) > { > this.clustered = clustered; > this.analyzer = analyzer; > this.request = request; > this.perthreadManager = perthreadManager; > + this.sessionLockingEnabled = sessionLockingEnabled; > } > > public Session getSession(boolean create) > @@ -140,6 +156,11 @@ public class TapestrySessionFactoryImpl implements > TapestrySessionFactory > > private SessionLock lockForSession(HttpSession session) > { > + if (!sessionLockingEnabled) > + { > + return NO_OP_LOCK; > + } > + > // Because WeakHashMap does not look thread safe to me, we use an > exclusive > // lock. > mapLock.lock(); > > http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/b7b44b8f/tapestry-core/src/main/java/org/apache/tapestry5/services/TapestryModule.java > ---------------------------------------------------------------------- > diff --git > a/tapestry-core/src/main/java/org/apache/tapestry5/services/TapestryModule.java > > b/tapestry-core/src/main/java/org/apache/tapestry5/services/TapestryModule.java > index a5b720f..a75dbc2 100644 > --- > a/tapestry-core/src/main/java/org/apache/tapestry5/services/TapestryModule.java > +++ > b/tapestry-core/src/main/java/org/apache/tapestry5/services/TapestryModule.java > @@ -2214,6 +2214,8 @@ public final class TapestryModule > configuration.add(SymbolConstants.HMAC_PASSPHRASE, ""); > > configuration.add(SymbolConstants.BOOTSTRAP_ROOT, > "${tapestry.asset.root}/bootstrap_2_1_1"); > + > + configuration.add(SymbolConstants.SESSION_LOCKING_ENABLED, true); > } > > /** > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tapestry.apache.org For additional commands, e-mail: dev-h...@tapestry.apache.org
