Author: markt
Date: Tue Jun 23 18:32:01 2009
New Revision: 787779
URL: http://svn.apache.org/viewvc?rev=787779&view=rev
Log:
Fix additional concurrency issues identified in
https://issues.apache.org/bugzilla/show_bug.cgi?id=43343
Modified:
tomcat/tc6.0.x/trunk/ (props changed)
tomcat/tc6.0.x/trunk/STATUS.txt
tomcat/tc6.0.x/trunk/java/org/apache/catalina/session/PersistentManagerBase.java
tomcat/tc6.0.x/trunk/java/org/apache/catalina/valves/PersistentValve.java
tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml
tomcat/tc6.0.x/trunk/webapps/docs/config/manager.xml
Propchange: tomcat/tc6.0.x/trunk/
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Tue Jun 23 18:32:01 2009
@@ -1 +1 @@
-/tomcat/trunk:601180,606992,612607,630314,640888,652744,653247,673796,673820,683982,684001,684081,684234,684269-684270,685177,687503,687645,689402,690781,691392,691805,692748,693378,694992,695053,695311,696780,696782,698012,698227,698236,698613,699427,699634,701355,709294,709811,709816,710063,710066,710125,710205,711126,711600,712461,712467,718360,719119,719124,719602,719626,719628,720046,720069,721040,721286,721708,721886,723404,723738,726052,727303,728032,728768,728947,729057,729567,729569,729571,729681,729809,729815,729934,730250,730590,731651,732859,732863,734734,740675,740684,742677,742697,742714,744160,744238,746321,746384,746425,747834,747863,748344,750258,750291,750921,751286-751287,751289,751295,753039,757335,757774,758365,758596,758616,758664,759074,761601,762868,762929,762936-762937,763166,763183,763193,763228,763262,763298,763302,763325,763599,763611,763654,763681,763706,764985,764997,765662,768335,769979,770716,770809,770876,776921,776924,776935,776945,777464,77
7466,777576,777625,778379,781528,781779,782145,782791,783316,783696,783756,784614,785688
+/tomcat/trunk:601180,606992,612607,630314,640888,652744,653247,673796,673820,683982,684001,684081,684234,684269-684270,685177,687503,687645,689402,690781,691392,691805,692748,693378,694992,695053,695311,696780,696782,698012,698227,698236,698613,699427,699634,701355,709294,709811,709816,710063,710066,710125,710205,711126,711600,712461,712467,718360,719119,719124,719602,719626,719628,720046,720069,721040,721286,721708,721886,723404,723738,726052,727303,728032,728768,728947,729057,729567,729569,729571,729681,729809,729815,729934,730250,730590,731651,732859,732863,734734,740675,740684,742677,742697,742714,744160,744238,746321,746384,746425,747834,747863,748344,750258,750291,750921,751286-751287,751289,751295,753039,757335,757774,758365,758596,758616,758664,759074,761601,762868,762929,762936-762937,763166,763183,763193,763228,763262,763298,763302,763325,763599,763611,763654,763681,763706,764985,764997,765662,768335,769979,770716,770809,770876,776921,776924,776935,776945,777464,77
7466,777576,777625,778379,778523-778524,781528,781779,782145,782791,783316,783696,783724,783756,784453,784602,784614,785688,786468,786667,787627,787770
Modified: tomcat/tc6.0.x/trunk/STATUS.txt
URL:
http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/STATUS.txt?rev=787779&r1=787778&r2=787779&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/STATUS.txt (original)
+++ tomcat/tc6.0.x/trunk/STATUS.txt Tue Jun 23 18:32:01 2009
@@ -93,63 +93,6 @@
+1: rjung, fhanik, markt,funkman
-1:
-* Fix for https://issues.apache.org/bugzilla/show_bug.cgi?id=43343
- Additional fixes for case B. Don't swap out session if one or more requests
- are currently using it.
- http://svn.apache.org/viewvc?rev=778523&view=rev
- http://svn.apache.org/viewvc?rev=778524&view=rev
- +1: markt,funkman
- +1: kkolinko: (
- good, I tested it and it fixes the issue,
- but I have the following comments:
-
- 1. It works only if StandardSession.ACTIVITY_CHECK is true.
- And that occurs only if one of the following properties is "true":
-
- org.apache.catalina.session.StandardSession.ACTIVITY_CHECK
- org.apache.catalina.STRICT_SERVLET_COMPLIANCE
-
- Both are "false" by default, and that results in NullPointerException
- because StandardSession.accessCount is null in that case.
-
- That is expected, and just needs to be documented.
- Though some IllegalStateException might be better than NPE.
-
- Also maybe direct access to session.accessCount (a protected field of
- a class in the same package) is not a good style.
-
- 2. In PersistentManagerBase.processMaxIdleSwaps():
- there is already the following line:
- "StandardSession session = (StandardSession) sessions[i];"
-
- Thus, there is no need for "if (sessions[i] instanceof StandardSession)"
check
- in your patch, and you can access "session" instead of "session[i]".
-
- I think that "instanceof StandardSession" is a requirement, like
- ACTIVITY_CHECK one above, and thus it is no need for "instanceof"
- check. Just do a cast.
-
- 3. PersistentValve class has "session.access()" call that I do not see
- how is balanced with "endAccess()". A reference to that session is just
lost
- and not stored in a Request (that would release it in its recycle()
method),
- so I think that additional "endAccess()" call is needed there. I have no
- experience with that class, though.
-
- )
- -1:
-* Additional patch based on review comments above:
- http://svn.apache.org/viewvc?rev=784453&view=rev
- +1: markt, fhanik
- +1: kkolinko (ok, comments 1&2 addressed; for PersistentValve (3.) I
- propose an additional patch below)
- -1:
-* Additional patch:
- Do not increment access counter in PersistentValve
- http://svn.apache.org/viewvc?rev=784602&view=rev
- +1: kkolinko, markt, fhanik
- -1:
-
-
* Dont try to report thread counts when using an executor from outside
http://people.apache.org/~fhanik/connector-thread-report.patch
+1: fhanik
Modified:
tomcat/tc6.0.x/trunk/java/org/apache/catalina/session/PersistentManagerBase.java
URL:
http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/java/org/apache/catalina/session/PersistentManagerBase.java?rev=787779&r1=787778&r2=787779&view=diff
==============================================================================
---
tomcat/tc6.0.x/trunk/java/org/apache/catalina/session/PersistentManagerBase.java
(original)
+++
tomcat/tc6.0.x/trunk/java/org/apache/catalina/session/PersistentManagerBase.java
Tue Jun 23 18:32:01 2009
@@ -814,6 +814,9 @@
((StandardSession)session).tellNew();
add(session);
((StandardSession)session).activate();
+ // endAccess() to ensure timeouts happen correctly.
+ // access() to keep access count correct or it will end up negative
+ session.access();
session.endAccess();
return (session);
@@ -1050,6 +1053,11 @@
int timeIdle = // Truncate, do not round up
(int) ((timeNow - session.getLastAccessedTime()) /
1000L);
if (timeIdle > maxIdleSwap && timeIdle > minIdleSwap) {
+ if (session.accessCount != null &&
+ session.accessCount.get() > 0) {
+ // Session is currently being accessed - skip
it
+ continue;
+ }
if (log.isDebugEnabled())
log.debug(sm.getString
("persistentManager.swapMaxIdle",
@@ -1090,16 +1098,22 @@
long timeNow = System.currentTimeMillis();
for (int i = 0; i < sessions.length && toswap > 0; i++) {
- synchronized (sessions[i]) {
+ StandardSession session = (StandardSession) sessions[i];
+ synchronized (session) {
int timeIdle = // Truncate, do not round up
- (int) ((timeNow - sessions[i].getLastAccessedTime()) /
1000L);
+ (int) ((timeNow - session.getLastAccessedTime()) / 1000L);
if (timeIdle > minIdleSwap) {
+ if (session.accessCount != null &&
+ session.accessCount.get() > 0) {
+ // Session is currently being accessed - skip it
+ continue;
+ }
if(log.isDebugEnabled())
log.debug(sm.getString
("persistentManager.swapTooManyActive",
- sessions[i].getIdInternal(), new
Integer(timeIdle)));
+ session.getIdInternal(), new Integer(timeIdle)));
try {
- swapOut(sessions[i]);
+ swapOut(session);
} catch (IOException e) {
; // This is logged in writeSession()
}
Modified:
tomcat/tc6.0.x/trunk/java/org/apache/catalina/valves/PersistentValve.java
URL:
http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/java/org/apache/catalina/valves/PersistentValve.java?rev=787779&r1=787778&r2=787779&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/java/org/apache/catalina/valves/PersistentValve.java
(original)
+++ tomcat/tc6.0.x/trunk/java/org/apache/catalina/valves/PersistentValve.java
Tue Jun 23 18:32:01 2009
@@ -137,6 +137,7 @@
manager.add(session);
// ((StandardSession)session).activate();
session.access();
+ session.endAccess();
}
}
}
Modified: tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml
URL:
http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml?rev=787779&r1=787778&r2=787779&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml Tue Jun 23 18:32:01 2009
@@ -49,6 +49,10 @@
(markt/idarwin)
</update>
<fix>
+ <bug>43343</bug>: Fix additional concurrency issues identified with the
+ persistent session manager. (markt)
+ </fix>
+ <fix>
<bug>46908</bug>: Try and support java encoding names when using an xml
parser provided via the endorsed mechanism. (markt)
</fix>
Modified: tomcat/tc6.0.x/trunk/webapps/docs/config/manager.xml
URL:
http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/webapps/docs/config/manager.xml?rev=787779&r1=787778&r2=787779&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/webapps/docs/config/manager.xml (original)
+++ tomcat/tc6.0.x/trunk/webapps/docs/config/manager.xml Tue Jun 23 18:32:01
2009
@@ -165,6 +165,12 @@
has not been thoroughly tested, and should be considered experimental!
</strong></em></p>
+ <p><strong>NOTE:</strong> You must set either the
+ <code>org.apache.catalina.session.StandardSession.ACTIVITY_CHECK</code> or
+ <code>org.apache.catalina.STRICT_SERVLET_COMPLIANCE</code>
+ <a href="systemprops.html">system properties</a> to <code>true</code> for
+ the persistent manager to work correctly.</p>
+
<p>The persistent implementation of <strong>Manager</strong> is
<strong>org.apache.catalina.session.PersistentManager</strong>. In
addition to the usual operations of creating and deleting sessions, a
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
