Remy Maucherat wrote:
Jess Holle wrote:
And finally, here are the patches against 5.5.6 (same as HEAD at this moment in this case).
Ok, but I still dislike the patch ;)
I'd appreciate comments as to what's wrong with the patch :-)
I am clearly new to this area of Tomcat, but there were clear issues in this area of the code that I cannot ignore if I am to depend on PersistentManager.
Primary issues encountered were:
* Race condition previous noted by "FIXME". o There were no guarantees that a session would not be serialized to disk, passivated, and recyled while it was being used. o There were variations on this. I fixed the more obvious ones by the admittedly unfortunately synchronization on access() and around use of swapOut() in conjunction with checks on accessCount. The less obvious case required a change access(), Request, etc, to allow an already referenced (but not accessed) session which has been swapped out (and thus recycled) to be swapped back in within access() as necessary/appropriate. * Lack of any sort of LRU prioritization to what is passivated when the maximum number of active sessions has been exceeded. o I replaced usage of HashMap with LinkedHashMap in PersistentManagerBase (only). * Extremely inefficient behavior when trying to expire passivated sessions. o All passivated sessions were fully deserialized back into memory to check if they were stale upon each and every processExpires() execution. For large numbers of large sessions (the whole reason you'd bother with passivation!), this will be a large drain on performance. o My changes make the modification time more accessible in FileStore and make more efficient staleness checks in both FileStore and JDBCStore.
Additionally there was a discrepancy between lastAccessedTime and thisAccessedTime usage in normal isValid(), etc, and the behavior of PersistentManagerBase, etc
In all of these cases I may have misunderstood something or overlooked more optimal solutions. I would like to see all of these issues addressed in the best fashion possible -- and if possible in the standard distribution of Tomcat rather than just in ours. I am thus more than open to suggestions for improvement. The issue is that right now there are clear, unaddressed issues which I believe I have at least improved upon though certainly not perfected.
-- Jess Holle
P.S. I assumed Manager, etc, interfaces should not be extended, hence my introduction of some new methods at ManagerBase and my instanceof checks. This may have been a bad assumption, but this can easily be amended as needed.