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.
