Details
-
Improvement
-
Status: Closed
-
Normal
-
Resolution: Fixed
-
2.10.01
-
None
-
None
Description
First of all, currently the JCRSessionConcurrencyValve seems to be only added to the page composer.
However, any request using sessionstateful jcr session must have synchronized processing, even only read requests. This is similar to the cms where this is handled by wicket.
Furthermore, currently, every jcr session, even when not a sessionstateful jcr session, is used in synchronized block in JCRSessionConcurrencyValve. This is not needed. Most likely, it cannot really harm, but when a jcr session comes from a pool, no other request can get this jcr session as well. So, no synchronisation is needed! Also, synchronizing jcr session from the session pool is very bad: The internal commons pool will be blocked all the time when its background thread runs checking jcr sessions! This is not right!
So, I will make the following changes:
1) JCRSessionConcurrencyValve will be renamed to JCRSessionStatefulConcurrencyValve because it is only for stateful sessions
2) Onlywhen the session is sessionstateful, we will use synchronized
Furthermore, I will look into the refresh logic for sessionstateful jcr sessions. I doubt whether we should have sessionstateful refreshed in the Cleanup valve, as this can cause race conditions because another request for the same jcr session is not locked anymore in the JCRSessionStatefulConcurrencyValve when the Cleanup valve is invoked. I might move the refresh logic for sessionstateful out of the Cleanup valve, and into the synchronized block as this is crucial to avoid race conditions (for example, the cleanup thread refreshes a jcr session, but the page composer uses the same jcr session concurrently and just added a node, but not yet saved it)