Incomplete change of session ID after Tomcat failover (with jvmRoute + securityConstraint/Valve)

[GoogleCodeExporter]

Observed for the TC 7 branch, but likely not restricted to that.

Directly after a Tomcat failover the first request R1B on the new node B 
triggers changing the session ID (e.g. in cookie) but still operates on the old 
session. After that request there are two sessions, one with the old ID, one 
with the new one (see the session list in the Tomcat manager webapp).

The next request R2B - still send to the failover Tomcat node B - now uses the 
"new" session, which has the same attributes as the old one, except that the 
values are the ones from the time the id was changed, i.e. when R1B started. 
Any changes done by R1B are lost.

Correct handling of ID changes can be studied e.g. in 
org.apache.catalina.ha.session.JvmRouteBinderValve. In method 
SessionTrackerValve.changeRequestedSessionId() in addition to 
request.changeSessionId( newSessionId ) there should be called something like:

Session catalinaSession = request.getSessionInternal(false);
catalinaSession.setId(newSessionId);

At least this fixes the dublicate sessions after request R1B and the missing 
attribute updates.

Original issue reported on code.google.com by rainer.j...@kippdata.de on 23 Mar 2011 at 1:56

[GoogleCodeExporter]

Simple "session.jsp" JSP to reproduce

<pre>
<% String attributeName = "COUNTER"; %>
RSID:   <%= request.getRequestedSessionId() %>
Valid:  <%= request.isRequestedSessionIdValid() %>
SID:    <%= request.getSession().getId() %>
Primary:<%= request.getAttribute("isPrimary") %>
Value before increment: <%= session.getAttribute(attributeName) %>
<%Object o = session.getAttribute(attributeName);
if (o==null) {
    session.setAttribute(attributeName,1);
} else {
    String s=o.toString();
    session.setAttribute(attributeName,Integer.parseInt(s)+1);
}
%>
Value after increment: <%= session.getAttribute(attributeName) %>

<a href="session.jsp;jsessionid=<%=session.getId()%>">Reuse Session</a>
</pre>

Original comment by rainer.j...@kippdata.de on 23 Mar 2011 at 2:00

[GoogleCodeExporter]

Tomcat failover is handled in 
MemcachedBackupSessionManager.changeSessionIdOnTomcatFailover and 
handleSessionTakeOver, this also sets the newSessionId on the session.
I can imagine that it's directly related to issue 93, as it does not check if 
the session perhaps has already been loaded and removes it in this case.

Original comment by martin.grotzke on 23 Mar 2011 at 8:14

• Changed title: Incomplete change of session ID after Tomcat failover (with jvmRoute set)
• Changed state: Accepted

[GoogleCodeExporter]

Hmmm, the TC cluster valve uses session.setId(), your manager uses 
session.setIdInternal. The former removes the session from the manager, then 
changes the id and finally adds it again to the manager. At the end it calls 
tellNew(). You internal method setIdInternal() only sets the id.

I removed my changes to the SessionTrackerValve and replaced 
session.setIdInternal() in 
MemcachedBackupSessionManager.handleSessionTakeOver(). by sesion.setId(). 
Surprise: it does *not* work there. Using the testing JSP after failover I 
still get the same behaviour.

Note that when using the above JSP the first request on the failover node gets 
for request.getRequestedSessionId() the new id and for 
request.getSession().getId() the old id. Both sessions are still in the manager 
and the attribute update actually happen to the old session.

Regards,

Rainer

Original comment by rainer.j...@kippdata.de on 24 Mar 2011 at 9:00

[GoogleCodeExporter]

That setId is not used is intentional: as manager.remove(Session) would remove 
the session also from memcached if it's not handled otherwise.

What should be done is to replace session.setIdInternal by 
session.setIdForRelocate, which does all the "magic". This might already do the 
trick.

Original comment by martin.grotzke on 24 Mar 2011 at 2:01

[GoogleCodeExporter]

Btw, if you want to provide patches you can also use github + fork + 
pull-requests.

Original comment by martin.grotzke on 24 Mar 2011 at 2:01

[GoogleCodeExporter]

Unfortunately replacing session.setIdInternal( newSessionId ) by         
session.setIdForRelocate( newSessionId ) inside 
MemcachedBackupSessionManager.handleSessionTakeOver() doesn't work for me. 
Still the same behaviour. Can you reproduce it?

Concerning git: yeah, just trying to get used to it. Once patches get bigger 
than just a line or two ...

Regards,

Rainer

Original comment by rainer.j...@kippdata.de on 24 Mar 2011 at 3:06

[GoogleCodeExporter]

Yes, I can reproduce it when I add a security-constraint to my web.xml (which 
adds the additional valve) and set a jvmRoute on the engine.

There's a quick fix possible (in changeSessionIdOnTomcatFailover before loading 
from memcached we must check the local sessions map, in handleSessionTakeOver 
there must be added s.th. like sessions.remove( origSessionId )). However this 
would break HttpSessionActivationListeners as they'd already got bound in 
initial findSession, therefore the findSession and 
changeSessionIdOnTomcatFailover (including Set-Cookie) must be brought together.
I hope I find time to fix this the next days.

Original comment by martin.grotzke on 24 Mar 2011 at 6:50

• Changed title: Incomplete change of session ID after Tomcat failover (with jvmRoute + securityConstraint/Valve)

[GoogleCodeExporter]

Fixed in master and tomcat7 branch.

Original comment by martin.grotzke on 25 Mar 2011 at 11:23

• Changed state: Fixed
• Added labels: Milestone-1.4.1

[GoogleCodeExporter]

Fix validated on my TC 7 installation.
Thanks!

Original comment by rainer.j...@kippdata.de on 31 Mar 2011 at 4:04