Author: markt Date: Thu Jun 18 09:25:00 2009 New Revision: 785983 URL: http://svn.apache.org/viewvc?rev=785983&view=rev Log: Expand sync within rotatable block to fix a couple of issues: - fileDateFormatter is a SimpleDateFormat which is not thread safe - the rotationLastChecked needs to be volatile to ensure we don't execute the sync'd block multiple times Although this is a sync on 'this' in log which gets called for every request: - a similar sync occurs in getDate() for every request with minimal performance impact - microbenchmarks suggest that a sync on 'this' has similar performance to using ThreadLocals Based on kkolinko's patch for Tomcat 5.5.x Note there remains an issue with writing to the log if the log files happens to be in the process of rotating
Modified: tomcat/trunk/java/org/apache/catalina/valves/AccessLogValve.java Modified: tomcat/trunk/java/org/apache/catalina/valves/AccessLogValve.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/valves/AccessLogValve.java?rev=785983&r1=785982&r2=785983&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/catalina/valves/AccessLogValve.java (original) +++ tomcat/trunk/java/org/apache/catalina/valves/AccessLogValve.java Thu Jun 18 09:25:00 2009 @@ -295,7 +295,7 @@ /** * Instant when the log daily rotation was last checked. */ - private long rotationLastChecked = 0L; + private volatile long rotationLastChecked = 0L; /** * Do we check for log file existence? Helpful if an external @@ -649,19 +649,21 @@ // Only do a logfile switch check once a second, max. long systime = System.currentTimeMillis(); if ((systime - rotationLastChecked) > 1000) { - - rotationLastChecked = systime; - - // Check for a change of date - String tsDate = fileDateFormatter.format(new Date(systime)); - - // If the date has changed, switch log files - if (!dateStamp.equals(tsDate)) { - synchronized (this) { + synchronized(this) { + if ((systime - rotationLastChecked) > 1000) { + rotationLastChecked = systime; + + String tsDate; + // Check for a change of date + tsDate = fileDateFormatter.format(new Date(systime)); + + // If the date has changed, switch log files if (!dateStamp.equals(tsDate)) { - close(); - dateStamp = tsDate; - open(); + if (!dateStamp.equals(tsDate)) { + close(); + dateStamp = tsDate; + open(); + } } } } --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org