You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by kk...@apache.org on 2009/07/10 01:38:00 UTC
svn commit: r792748 - in /tomcat:
container/tc5.5.x/catalina/src/share/org/apache/catalina/valves/
container/tc5.5.x/webapps/docs/ current/tc5.5.x/
Author: kkolinko
Date: Thu Jul 9 23:37:59 2009
New Revision: 792748
URL: http://svn.apache.org/viewvc?rev=792748&view=rev
Log:
Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=38483
Make access log valves thread safe
Modified:
tomcat/container/tc5.5.x/catalina/src/share/org/apache/catalina/valves/AccessLogValve.java
tomcat/container/tc5.5.x/catalina/src/share/org/apache/catalina/valves/ExtendedAccessLogValve.java
tomcat/container/tc5.5.x/catalina/src/share/org/apache/catalina/valves/FastCommonAccessLogValve.java
tomcat/container/tc5.5.x/webapps/docs/changelog.xml
tomcat/current/tc5.5.x/STATUS.txt
Modified: tomcat/container/tc5.5.x/catalina/src/share/org/apache/catalina/valves/AccessLogValve.java
URL: http://svn.apache.org/viewvc/tomcat/container/tc5.5.x/catalina/src/share/org/apache/catalina/valves/AccessLogValve.java?rev=792748&r1=792747&r2=792748&view=diff
==============================================================================
--- tomcat/container/tc5.5.x/catalina/src/share/org/apache/catalina/valves/AccessLogValve.java (original)
+++ tomcat/container/tc5.5.x/catalina/src/share/org/apache/catalina/valves/AccessLogValve.java Thu Jul 9 23:37:59 2009
@@ -232,69 +232,103 @@
* A date formatter to format a Date into a date in the format
* "yyyy-MM-dd".
*/
- private SimpleDateFormat dateFormatter = null;
-
-
- /**
- * A date formatter to format Dates into a day string in the format
- * "dd".
- */
- private SimpleDateFormat dayFormatter = null;
-
-
- /**
- * A date formatter to format a Date into a month string in the format
- * "MM".
- */
- private SimpleDateFormat monthFormatter = null;
-
-
- /**
- * Time taken formatter for 3 decimal places.
- */
- private DecimalFormat timeTakenFormatter = null;
-
-
- /**
- * A date formatter to format a Date into a year string in the format
- * "yyyy".
- */
- private SimpleDateFormat yearFormatter = null;
-
-
- /**
- * A date formatter to format a Date into a time in the format
- * "kk:mm:ss" (kk is a 24-hour representation of the hour).
- */
- private SimpleDateFormat timeFormatter = null;
+ private SimpleDateFormat fileDateFormatter = null;
/**
* The system timezone.
*/
- private TimeZone timezone = null;
+ private static final TimeZone timezone;
/**
* The time zone offset relative to GMT in text form when daylight saving
* is not in operation.
*/
- private String timeZoneNoDST = null;
+ private static final String timeZoneNoDST;
/**
* The time zone offset relative to GMT in text form when daylight saving
* is in operation.
*/
- private String timeZoneDST = null;
-
-
+ private static final String timeZoneDST;
+
+ static {
+ timezone = TimeZone.getDefault();
+ timeZoneNoDST = calculateTimeZoneOffset(timezone.getRawOffset());
+ int offset = timezone.getDSTSavings();
+ timeZoneDST = calculateTimeZoneOffset(timezone.getRawOffset()+offset);
+ }
+
/**
* The system time when we last updated the Date that this valve
* uses for log lines.
*/
- private Date currentDate = null;
+ private static class AccessDateStruct {
+ private Date currentDate = new Date();
+ private String currentDateString = null;
+ private SimpleDateFormat dayFormatter = new SimpleDateFormat("dd");
+ private SimpleDateFormat monthFormatter = new SimpleDateFormat("MM");
+ private SimpleDateFormat yearFormatter = new SimpleDateFormat("yyyy");
+ private SimpleDateFormat timeFormatter = new SimpleDateFormat("HH:mm:ss");
+ private DecimalFormat timeTakenFormatter;
+
+ public AccessDateStruct() {
+ dayFormatter.setTimeZone(timezone);
+ monthFormatter.setTimeZone(timezone);
+ yearFormatter.setTimeZone(timezone);
+ timeFormatter.setTimeZone(timezone);
+ }
+ public Date getDate() {
+ // Only update the Date once per second, max.
+ long systime = System.currentTimeMillis();
+ if ((systime - currentDate.getTime()) > 1000) {
+ currentDate.setTime(systime);
+ currentDateString = null;
+ }
+ return currentDate;
+ }
+
+ /**
+ * Format current date and time in Common Log Format format. That is:
+ * "[dd/MMM/yyyy:HH:mm:ss Z]"
+ */
+ public String getCurrentDateString() {
+ if (currentDateString == null) {
+ StringBuffer current = new StringBuffer(32);
+ current.append('[');
+ current.append(dayFormatter.format(currentDate));
+ current.append('/');
+ current.append(lookup(monthFormatter.format(currentDate)));
+ current.append('/');
+ current.append(yearFormatter.format(currentDate));
+ current.append(':');
+ current.append(timeFormatter.format(currentDate));
+ current.append(' ');
+ current.append(getTimeZone(currentDate));
+ current.append(']');
+ currentDateString = current.toString();
+ }
+ return currentDateString;
+ }
+
+ /**
+ * Format the time taken value for 3 decimal places.
+ */
+ public String formatTimeTaken(long time) {
+ if (timeTakenFormatter == null) {
+ timeTakenFormatter = new DecimalFormat("0.000");
+ }
+ return timeTakenFormatter.format(time/1000d);
+ }
+ }
+ private static ThreadLocal currentDateStruct = new ThreadLocal() {
+ protected Object initialValue() {
+ return new AccessDateStruct();
+ }
+ };
/**
* When formatting log lines, we often use strings like this one (" ").
@@ -311,7 +345,7 @@
/**
* Instant when the log daily rotation was last checked.
*/
- private long rotationLastChecked = 0L;
+ private volatile long rotationLastChecked = 0L;
/**
@@ -555,7 +589,8 @@
}
- Date date = getDate();
+ AccessDateStruct struct = (AccessDateStruct) currentDateStruct.get();
+ Date date = struct.getDate();
StringBuffer result = new StringBuffer();
// Check to see if we should log using the "common" access log pattern
@@ -577,18 +612,9 @@
result.append(space);
}
- result.append("[");
- result.append(dayFormatter.format(date)); // Day
- result.append('/');
- result.append(lookup(monthFormatter.format(date))); // Month
- result.append('/');
- result.append(yearFormatter.format(date)); // Year
- result.append(':');
- result.append(timeFormatter.format(date)); // Time
- result.append(space);
- result.append(getTimeZone(date)); // Time Zone
- result.append("] \"");
+ result.append(struct.getCurrentDateString());
+ result.append(" \"");
result.append(request.getMethod());
result.append(space);
result.append(request.getRequestURI());
@@ -657,10 +683,10 @@
} else {
//D'oh - end of string - pretend we never did this
//and do processing the "old way"
- result.append(replace(ch, date, request, response, time));
+ result.append(replace(ch, struct, request, response, time));
}
} else {
- result.append(replace(ch, date, request, response,time ));
+ result.append(replace(ch, struct, request, response,time ));
}
replace = false;
} else if (ch == '%') {
@@ -707,17 +733,15 @@
// Only do a logfile switch check once a second, max.
long systime = System.currentTimeMillis();
if ((systime - rotationLastChecked) > 1000) {
-
- // We need a new currentDate
- currentDate = new Date(systime);
- rotationLastChecked = systime;
-
- // Check for a change of date
- String tsDate = dateFormatter.format(currentDate);
-
- // If the date has changed, switch log files
- if (!dateStamp.equals(tsDate)) {
- synchronized (this) {
+ synchronized(this) {
+ 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)) {
close();
dateStamp = tsDate;
@@ -725,13 +749,14 @@
}
}
}
-
}
}
// Log this message
- if (writer != null) {
- writer.println(message);
+ synchronized(this) {
+ if (writer != null) {
+ writer.println(message);
+ }
}
}
@@ -743,7 +768,7 @@
*
* @param month Month number ("01" .. "12").
*/
- private String lookup(String month) {
+ private static String lookup(String month) {
int index;
try {
@@ -790,12 +815,12 @@
* Return the replacement text for the specified pattern character.
*
* @param pattern Pattern character identifying the desired text
- * @param date the current Date so that this method doesn't need to
- * create one
+ * @param struct the object containing current Date so that this method
+ * doesn't need to create one
* @param request Request being processed
* @param response Response being processed
*/
- private String replace(char pattern, Date date, Request request,
+ private String replace(char pattern, AccessDateStruct struct, Request request,
Response response, long time) {
String value = null;
@@ -868,20 +893,9 @@
else
value = MARK_EMPTY;
} else if (pattern == 't') {
- StringBuffer temp = new StringBuffer("[");
- temp.append(dayFormatter.format(date)); // Day
- temp.append('/');
- temp.append(lookup(monthFormatter.format(date))); // Month
- temp.append('/');
- temp.append(yearFormatter.format(date)); // Year
- temp.append(':');
- temp.append(timeFormatter.format(date)); // Time
- temp.append(' ');
- temp.append(getTimeZone(date)); // Timezone
- temp.append(']');
- value = temp.toString();
+ value = struct.getCurrentDateString();
} else if (pattern == 'T') {
- value = timeTakenFormatter.format(time/1000d);
+ value = struct.formatTimeTaken(time);
} else if (pattern == 'u') {
if (request != null)
value = request.getRemoteUser();
@@ -990,40 +1004,16 @@
}
- /**
- * This method returns a Date object that is accurate to within one
- * second. If a thread calls this method to get a Date and it's been
- * less than 1 second since a new Date was created, this method
- * simply gives out the same Date again so that the system doesn't
- * spend time creating Date objects unnecessarily.
- *
- * @return Date
- */
- private Date getDate() {
- if(currentDate == null) {
- currentDate = new Date();
- } else {
- // Only create a new Date once per second, max.
- long systime = System.currentTimeMillis();
- if ((systime - currentDate.getTime()) > 1000) {
- currentDate = new Date(systime);
- }
- }
-
- return currentDate;
- }
-
-
- private String getTimeZone(Date date) {
+ private static String getTimeZone(Date date) {
if (timezone.inDaylightTime(date)) {
return timeZoneDST;
} else {
return timeZoneNoDST;
}
}
-
-
- private String calculateTimeZoneOffset(long offset) {
+
+
+ private static String calculateTimeZoneOffset(long offset) {
StringBuffer tz = new StringBuffer();
if ((offset<0)) {
tz.append(MARK_EMPTY);
@@ -1102,27 +1092,14 @@
lifecycle.fireLifecycleEvent(START_EVENT, null);
started = true;
- // Initialize the timeZone, Date formatters, and currentDate
- timezone = TimeZone.getDefault();
- timeZoneNoDST = calculateTimeZoneOffset(timezone.getRawOffset());
- int offset = timezone.getDSTSavings();
- timeZoneDST = calculateTimeZoneOffset(timezone.getRawOffset()+offset);
-
+ // Initialize formatters, and dateStamp
if (fileDateFormat==null || fileDateFormat.length()==0)
fileDateFormat = "yyyy-MM-dd";
- dateFormatter = new SimpleDateFormat(fileDateFormat);
- dateFormatter.setTimeZone(timezone);
- dayFormatter = new SimpleDateFormat("dd");
- dayFormatter.setTimeZone(timezone);
- monthFormatter = new SimpleDateFormat("MM");
- monthFormatter.setTimeZone(timezone);
- yearFormatter = new SimpleDateFormat("yyyy");
- yearFormatter.setTimeZone(timezone);
- timeFormatter = new SimpleDateFormat("HH:mm:ss");
- timeFormatter.setTimeZone(timezone);
- currentDate = new Date();
- dateStamp = dateFormatter.format(currentDate);
- timeTakenFormatter = new DecimalFormat("0.000");
+ synchronized (this) {
+ fileDateFormatter = new SimpleDateFormat(fileDateFormat);
+ fileDateFormatter.setTimeZone(timezone);
+ dateStamp = fileDateFormatter.format(new Date());
+ }
open();
Modified: tomcat/container/tc5.5.x/catalina/src/share/org/apache/catalina/valves/ExtendedAccessLogValve.java
URL: http://svn.apache.org/viewvc/tomcat/container/tc5.5.x/catalina/src/share/org/apache/catalina/valves/ExtendedAccessLogValve.java?rev=792748&r1=792747&r2=792748&view=diff
==============================================================================
--- tomcat/container/tc5.5.x/catalina/src/share/org/apache/catalina/valves/ExtendedAccessLogValve.java (original)
+++ tomcat/container/tc5.5.x/catalina/src/share/org/apache/catalina/valves/ExtendedAccessLogValve.java Thu Jul 9 23:37:59 2009
@@ -26,10 +26,12 @@
import java.net.InetAddress;
import java.net.URLEncoder;
import java.text.DecimalFormat;
+import java.text.DecimalFormatSymbols;
import java.text.SimpleDateFormat;
import java.util.Date;
import java.util.Iterator;
import java.util.LinkedList;
+import java.util.Locale;
import java.util.TimeZone;
import javax.servlet.ServletException;
@@ -186,10 +188,9 @@
/**
- * The as-of date for the currently open log file, or a zero-length
- * string if there is no open log file.
+ * The as-of date for the currently open log file.
*/
- private String dateStamp = "";
+ private String dateStamp = null;
/**
@@ -205,23 +206,9 @@
/**
- * A date formatter to format a Date into a date in the format
- * "yyyy-MM-dd".
+ * The Date that is used to calculate the dateStamp for the file.
*/
- private SimpleDateFormat dateFormatter = null;
-
-
- /**
- * A date formatter to format a Date into a time in the format
- * "kk:mm:ss" (kk is a 24-hour representation of the hour).
- */
- private SimpleDateFormat timeFormatter = null;
-
-
- /**
- * Time taken formatter for 3 decimal places.
- */
- private DecimalFormat timeTakenFormatter = null;
+ private Date dateStampDate = new Date();
/**
@@ -253,18 +240,10 @@
private File currentLogFile = null;
-
- /**
- * The system time when we last updated the Date that this valve
- * uses for log lines.
- */
- private Date currentDate = null;
-
-
/**
* Instant when the log daily rotation was last checked.
*/
- private long rotationLastChecked = 0L;
+ private volatile long rotationLastChecked = 0L;
/**
@@ -316,6 +295,127 @@
private String fileDateFormat = null;
+ /**
+ * Helper class that stores thread-local variables that are used to format a
+ * log line. Note, that an instance of this class is shared by all
+ * {@link ExtendedAccessLogValve} instances in the same thread.
+ */
+ private static class LogDateStruct {
+ /**
+ * The time for the current log line.
+ */
+ private Date date;
+
+ /**
+ * A date formatter to format a Date into a date in the format
+ * "yyyy-MM-dd".
+ */
+ private SimpleDateFormat dateFormatter;
+
+ /**
+ * Formatted date value.
+ */
+ private String dateString;
+
+ /**
+ * A date formatter to format a Date into a time in the format
+ * "kk:mm:ss" (kk is a 24-hour representation of the hour).
+ */
+ private SimpleDateFormat timeFormatter;
+
+ /**
+ * Formatted time value.
+ */
+ private String timeString;
+
+ /**
+ * Time taken formatter for 3 decimal places.
+ */
+ private DecimalFormat timeTakenFormatter;
+
+ /**
+ * GMT timezone is used to format time and date fields.
+ */
+ private static final TimeZone timezoneGMT = TimeZone.getTimeZone("GMT");
+
+ /**
+ * Formatting symbols for DecimalFormat. Should use dot (and not comma)
+ * as the decimal point.
+ */
+ private static final DecimalFormatSymbols decimalsUS = new DecimalFormatSymbols(
+ Locale.US);
+
+ public LogDateStruct() {
+ date = new Date();
+ }
+
+ /**
+ * Updates the current time value. The time is updated only to be
+ * accurate within one second.
+ *
+ * @param systime
+ */
+ public void setTime(long systime) {
+ // Only update once per second, max.
+ long diff = systime - date.getTime();
+ if (diff > 1000 || diff < -1000) {
+ date.setTime(systime);
+ timeString = dateString = null;
+ }
+ }
+
+ /**
+ * Formats the date value.
+ *
+ * @return
+ */
+ public String formatDate() {
+ if (dateString == null) {
+ if (dateFormatter == null) {
+ dateFormatter = new SimpleDateFormat("yyyy-MM-dd");
+ dateFormatter.setTimeZone(timezoneGMT);
+ }
+ dateString = dateFormatter.format(date);
+ }
+ return dateString;
+ }
+
+ /**
+ * Formats the time value.
+ *
+ * @return
+ */
+ public String formatTime() {
+ if (timeString == null) {
+ if (timeFormatter == null) {
+ timeFormatter = new SimpleDateFormat("HH:mm:ss");
+ timeFormatter.setTimeZone(timezoneGMT);
+ }
+ timeString = timeFormatter.format(date);
+ }
+ return timeString;
+ }
+
+ /**
+ * Formats the time taken value.
+ *
+ * @return
+ */
+ public String formatTimeTaken(long runTime) {
+ if (timeTakenFormatter == null) {
+ timeTakenFormatter = new DecimalFormat("0.000", decimalsUS);
+ }
+ return timeTakenFormatter.format(runTime / 1000d);
+ }
+ }
+
+ private static final ThreadLocal logDateLocal = new ThreadLocal() {
+ protected Object initialValue() {
+ return new LogDateStruct();
+ }
+ };
+
+
// ------------------------------------------------------------- Properties
@@ -539,7 +639,9 @@
}
- Date date = getDate(endTime);
+ LogDateStruct logDate = (LogDateStruct) logDateLocal.get();
+ logDate.setTime(endTime);
+
StringBuffer result = new StringBuffer();
for (int i=0; fieldInfos!=null && i<fieldInfos.length; i++) {
@@ -580,11 +682,11 @@
break;
case FieldInfo.DATA_SPECIAL:
if (FieldInfo.SPECIAL_DATE==fieldInfos[i].location)
- result.append(dateFormatter.format(date));
+ result.append(logDate.formatDate());
else if (FieldInfo.SPECIAL_TIME_TAKEN==fieldInfos[i].location)
- result.append(timeTakenFormatter.format(runTime/1000d));
+ result.append(logDate.formatTimeTaken(runTime));
else if (FieldInfo.SPECIAL_TIME==fieldInfos[i].location)
- result.append(timeFormatter.format(date));
+ result.append(logDate.formatTime());
else if (FieldInfo.SPECIAL_BYTES==fieldInfos[i].location) {
long length = response.getContentCountLong() ;
if (length > 0)
@@ -604,7 +706,7 @@
result.append(fieldInfos[i].postWhiteSpace);
}
}
- log(result.toString(), date);
+ log(result.toString(), endTime);
}
@@ -630,8 +732,8 @@
}
/* Make sure date is correct */
- currentDate = new Date(System.currentTimeMillis());
- dateStamp = fileDateFormatter.format(currentDate);
+ dateStampDate.setTime(System.currentTimeMillis());
+ dateStamp = fileDateFormatter.format(dateStampDate);
open();
return true;
@@ -873,26 +975,23 @@
* has changed since the previous log call.
*
* @param message Message to be logged
- * @param date the current Date object (so this method doesn't need to
- * create a new one)
+ * @param systime The current time
*/
- private void log(String message, Date date) {
+ private void log(String message, long systime) {
- if (rotatable){
+ if (rotatable) {
// Only do a logfile switch check once a second, max.
- long systime = System.currentTimeMillis();
if ((systime - rotationLastChecked) > 1000) {
+ synchronized (this) {
+ systime = System.currentTimeMillis();
+ if ((systime - rotationLastChecked) > 1000) {
+ rotationLastChecked = systime;
+
+ // Check for a change of date
+ dateStampDate.setTime(systime);
+ String tsDate = fileDateFormatter.format(dateStampDate);
- // We need a new currentDate
- currentDate = new Date(systime);
- rotationLastChecked = systime;
-
- // Check for a change of date
- String tsDate = fileDateFormatter.format(currentDate);
-
- // If the date has changed, switch log files
- if (!dateStamp.equals(tsDate)) {
- synchronized (this) {
+ // If the date has changed, switch log files
if (!dateStamp.equals(tsDate)) {
close();
dateStamp = tsDate;
@@ -904,18 +1003,19 @@
}
/* In case something external rotated the file instead */
- if (checkExists){
+ if (checkExists) {
synchronized (this) {
- if (currentLogFile!=null && !currentLogFile.exists()) {
+ if (currentLogFile != null && !currentLogFile.exists()) {
try {
close();
- } catch (Throwable e){
+ } catch (Throwable e) {
log.info("at least this wasn't swallowed", e);
}
/* Make sure date is correct */
- currentDate = new Date(System.currentTimeMillis());
- dateStamp = fileDateFormatter.format(currentDate);
+ systime = System.currentTimeMillis();
+ dateStampDate.setTime(systime);
+ dateStamp = fileDateFormatter.format(dateStampDate);
open();
}
@@ -923,8 +1023,10 @@
}
// Log this message
- if (writer != null) {
- writer.println(message);
+ synchronized (this) {
+ if (writer != null) {
+ writer.println(message);
+ }
}
}
@@ -971,29 +1073,6 @@
}
- /**
- * This method returns a Date object that is accurate to within one
- * second. If a thread calls this method to get a Date and it's been
- * less than 1 second since a new Date was created, this method
- * simply gives out the same Date again so that the system doesn't
- * spend time creating Date objects unnecessarily.
- */
- private Date getDate(long systime) {
- /* Avoid extra call to System.currentTimeMillis(); */
- if (0==systime) {
- systime = System.currentTimeMillis();
- }
-
- // Only create a new Date once per second, max.
- if ((systime - currentDate.getTime()) > 1000) {
- currentDate.setTime(systime);
- }
-
- return currentDate;
-
- }
-
-
// ------------------------------------------------------ Lifecycle Methods
@@ -1049,18 +1128,15 @@
lifecycle.fireLifecycleEvent(START_EVENT, null);
started = true;
- // Initialize the timeZone, Date formatters, and currentDate
- TimeZone tz = TimeZone.getTimeZone("GMT");
- dateFormatter = new SimpleDateFormat("yyyy-MM-dd");
- dateFormatter.setTimeZone(tz);
- timeFormatter = new SimpleDateFormat("HH:mm:ss");
- timeFormatter.setTimeZone(tz);
- currentDate = new Date(System.currentTimeMillis());
+ // Initialize the fileDate formatter, and dateStamp
if (fileDateFormat==null || fileDateFormat.length()==0)
fileDateFormat = "yyyy-MM-dd";
- fileDateFormatter = new SimpleDateFormat(fileDateFormat);
- dateStamp = fileDateFormatter.format(currentDate);
- timeTakenFormatter = new DecimalFormat("0.000");
+
+ synchronized (this) {
+ fileDateFormatter = new SimpleDateFormat(fileDateFormat);
+ dateStampDate.setTime(System.currentTimeMillis());
+ dateStamp = fileDateFormatter.format(dateStampDate);
+ }
/* Everybody say ick ... ick */
try {
Modified: tomcat/container/tc5.5.x/catalina/src/share/org/apache/catalina/valves/FastCommonAccessLogValve.java
URL: http://svn.apache.org/viewvc/tomcat/container/tc5.5.x/catalina/src/share/org/apache/catalina/valves/FastCommonAccessLogValve.java?rev=792748&r1=792747&r2=792748&view=diff
==============================================================================
--- tomcat/container/tc5.5.x/catalina/src/share/org/apache/catalina/valves/FastCommonAccessLogValve.java (original)
+++ tomcat/container/tc5.5.x/catalina/src/share/org/apache/catalina/valves/FastCommonAccessLogValve.java Thu Jul 9 23:37:59 2009
@@ -140,6 +140,12 @@
/**
+ * Instant when the log daily rotation was last checked.
+ */
+ private volatile long rotationLastChecked = 0L;
+
+
+ /**
* The string manager for this package.
*/
private StringManager sm =
@@ -168,67 +174,78 @@
* A date formatter to format a Date into a date in the format
* "yyyy-MM-dd".
*/
- private SimpleDateFormat dateFormatter = null;
-
-
- /**
- * A date formatter to format Dates into a day string in the format
- * "dd".
- */
- private SimpleDateFormat dayFormatter = null;
-
-
- /**
- * A date formatter to format a Date into a month string in the format
- * "MM".
- */
- private SimpleDateFormat monthFormatter = null;
-
-
- /**
- * A date formatter to format a Date into a year string in the format
- * "yyyy".
- */
- private SimpleDateFormat yearFormatter = null;
-
-
- /**
- * A date formatter to format a Date into a time in the format
- * "kk:mm:ss" (kk is a 24-hour representation of the hour).
- */
- private SimpleDateFormat timeFormatter = null;
+ private SimpleDateFormat fileDateFormatter = null;
/**
* The system timezone.
*/
- private TimeZone timezone = null;
+ private static final TimeZone timezone;
/**
* The time zone offset relative to GMT in text form when daylight saving
* is not in operation.
*/
- private String timeZoneNoDST = null;
+ private static final String timeZoneNoDST;
/**
* The time zone offset relative to GMT in text form when daylight saving
* is in operation.
*/
- private String timeZoneDST = null;
+ private static final String timeZoneDST;
+ static {
+ timezone = TimeZone.getDefault();
+ timeZoneNoDST = calculateTimeZoneOffset(timezone.getRawOffset());
+ int offset = timezone.getDSTSavings();
+ timeZoneDST = calculateTimeZoneOffset(timezone.getRawOffset()+offset);
+ }
+
+ private static class AccessDateStruct {
+ private Date currentDate = new Date(0);
+ private String currentDateString = null;
+ private SimpleDateFormat dayFormatter = new SimpleDateFormat("dd");
+ private SimpleDateFormat monthFormatter = new SimpleDateFormat("MM");
+ private SimpleDateFormat yearFormatter = new SimpleDateFormat("yyyy");
+ private SimpleDateFormat timeFormatter = new SimpleDateFormat("HH:mm:ss");
+ public AccessDateStruct() {
+ dayFormatter.setTimeZone(timezone);
+ monthFormatter.setTimeZone(timezone);
+ yearFormatter.setTimeZone(timezone);
+ timeFormatter.setTimeZone(timezone);
+ }
+ public String getCurrentDateString() {
+ long systime = System.currentTimeMillis();
+ if ((systime - currentDate.getTime()) > 1000) {
+ currentDate.setTime(systime);
+ StringBuffer current = new StringBuffer(32);
+ current.append('[');
+ current.append(dayFormatter.format(currentDate));
+ current.append('/');
+ current.append(lookup(monthFormatter.format(currentDate)));
+ current.append('/');
+ current.append(yearFormatter.format(currentDate));
+ current.append(':');
+ current.append(timeFormatter.format(currentDate));
+ current.append(' ');
+ current.append(getTimeZone(currentDate));
+ current.append("] \"");
+ currentDateString = current.toString();
+ }
+ return currentDateString;
+ }
+ }
/**
* The system time when we last updated the Date that this valve
* uses for log lines.
*/
- private String currentDateString = null;
-
-
- /**
- * The instant where the date string was last updated.
- */
- private long currentDate = 0L;
+ private static ThreadLocal currentDateStruct = new ThreadLocal() {
+ protected Object initialValue() {
+ return new AccessDateStruct();
+ }
+ };
/**
@@ -459,8 +476,10 @@
* throwables will be caught and logged.
*/
public void backgroundProcess() {
- if (writer != null)
- writer.flush();
+ synchronized(this) {
+ if (writer != null)
+ writer.flush();
+ }
}
@@ -504,8 +523,8 @@
result.append(value);
result.append(space);
}
-
- result.append(getCurrentDateString());
+ AccessDateStruct struct = (AccessDateStruct) currentDateStruct.get();
+ result.append(struct.getCurrentDateString());
result.append(request.getMethod());
result.append(space);
@@ -580,9 +599,35 @@
*/
public void log(String message) {
- // Log this message
- if (writer != null) {
- writer.println(message);
+ // Check for log rotation
+ if (rotatable) {
+ // Only do a logfile switch check once a second, max.
+ long systime = System.currentTimeMillis();
+ if ((systime - rotationLastChecked) > 1000) {
+ synchronized(this) {
+ 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)) {
+ close();
+ dateStamp = tsDate;
+ open();
+ }
+ }
+ }
+ }
+ }
+
+ synchronized(this) {
+ // Log this message
+ if (writer != null) {
+ writer.println(message);
+ }
}
}
@@ -594,7 +639,7 @@
*
* @param month Month number ("01" .. "12").
*/
- private String lookup(String month) {
+ private static String lookup(String month) {
int index;
try {
@@ -638,80 +683,16 @@
}
- /**
- * This method returns a Date object that is accurate to within one
- * second. If a thread calls this method to get a Date and it's been
- * less than 1 second since a new Date was created, this method
- * simply gives out the same Date again so that the system doesn't
- * spend time creating Date objects unnecessarily.
- *
- * @return Date
- */
- private String getCurrentDateString() {
- // Only create a new Date once per second, max.
- long systime = System.currentTimeMillis();
- if ((systime - currentDate) > 1000) {
- synchronized (this) {
- // We don't care about being exact here: if an entry does get
- // logged as having happened during the previous second
- // it will not make any difference
- if ((systime - currentDate) > 1000) {
-
- // Format the new date
- Date date = new Date();
- StringBuffer result = new StringBuffer(32);
- result.append("[");
- // Day
- result.append(dayFormatter.format(date));
- result.append('/');
- // Month
- result.append(lookup(monthFormatter.format(date)));
- result.append('/');
- // Year
- result.append(yearFormatter.format(date));
- result.append(':');
- // Time
- result.append(timeFormatter.format(date));
- result.append(space);
- // Time zone
- result.append(getTimeZone(date));
- result.append("] \"");
-
- // Check for log rotation
- if (rotatable) {
- // Check for a change of date
- String tsDate = dateFormatter.format(date);
- // If the date has changed, switch log files
- if (!dateStamp.equals(tsDate)) {
- synchronized (this) {
- if (!dateStamp.equals(tsDate)) {
- close();
- dateStamp = tsDate;
- open();
- }
- }
- }
- }
-
- currentDateString = result.toString();
- currentDate = date.getTime();
- }
- }
- }
- return currentDateString;
- }
-
-
- private String getTimeZone(Date date) {
+ private static String getTimeZone(Date date) {
if (timezone.inDaylightTime(date)) {
return timeZoneDST;
} else {
return timeZoneNoDST;
}
}
-
-
- private String calculateTimeZoneOffset(long offset) {
+
+
+ private static String calculateTimeZoneOffset(long offset) {
StringBuffer tz = new StringBuffer();
if ((offset<0)) {
tz.append("-");
@@ -790,26 +771,14 @@
lifecycle.fireLifecycleEvent(START_EVENT, null);
started = true;
- // Initialize the timeZone, Date formatters, and currentDate
- timezone = TimeZone.getDefault();
- timeZoneNoDST = calculateTimeZoneOffset(timezone.getRawOffset());
- int offset = timezone.getDSTSavings();
- timeZoneDST = calculateTimeZoneOffset(timezone.getRawOffset()+offset);
-
+ // Initialize formatters, and dateStamp
if (fileDateFormat==null || fileDateFormat.length()==0)
fileDateFormat = "yyyy-MM-dd";
- dateFormatter = new SimpleDateFormat(fileDateFormat);
- dateFormatter.setTimeZone(timezone);
- dayFormatter = new SimpleDateFormat("dd");
- dayFormatter.setTimeZone(timezone);
- monthFormatter = new SimpleDateFormat("MM");
- monthFormatter.setTimeZone(timezone);
- yearFormatter = new SimpleDateFormat("yyyy");
- yearFormatter.setTimeZone(timezone);
- timeFormatter = new SimpleDateFormat("HH:mm:ss");
- timeFormatter.setTimeZone(timezone);
- currentDateString = getCurrentDateString();
- dateStamp = dateFormatter.format(new Date());
+ synchronized(this) {
+ fileDateFormatter = new SimpleDateFormat(fileDateFormat);
+ fileDateFormatter.setTimeZone(timezone);
+ dateStamp = fileDateFormatter.format(new Date());
+ }
open();
Modified: tomcat/container/tc5.5.x/webapps/docs/changelog.xml
URL: http://svn.apache.org/viewvc/tomcat/container/tc5.5.x/webapps/docs/changelog.xml?rev=792748&r1=792747&r2=792748&view=diff
==============================================================================
--- tomcat/container/tc5.5.x/webapps/docs/changelog.xml (original)
+++ tomcat/container/tc5.5.x/webapps/docs/changelog.xml Thu Jul 9 23:37:59 2009
@@ -210,6 +210,9 @@
It is already present in the classpath set by the manifest in bootstrap.jar.
(rjung)
</fix>
+ <fix>
+ <bug>38483</bug>: Thread safety issues in AccessLogValve classes. (kkolinko)
+ </fix>
</changelog>
</subsection>
<subsection name="Jasper">
Modified: tomcat/current/tc5.5.x/STATUS.txt
URL: http://svn.apache.org/viewvc/tomcat/current/tc5.5.x/STATUS.txt?rev=792748&r1=792747&r2=792748&view=diff
==============================================================================
--- tomcat/current/tc5.5.x/STATUS.txt (original)
+++ tomcat/current/tc5.5.x/STATUS.txt Thu Jul 9 23:37:59 2009
@@ -31,21 +31,6 @@
+1: markt, kkolinko, fhanik
-1:
-* Make access log valves thread safe
- https://issues.apache.org/bugzilla/show_bug.cgi?id=38483
- Patch for ExtendedAccessLogValve:
- http://people.apache.org/~kkolinko/patches/2009-06-30_tc55_EALV.patch
- +1: kkolinko, markt, fhanik
- -1:
-
-* Make access log valves thread safe
- https://issues.apache.org/bugzilla/show_bug.cgi?id=38483
- Patches for AccessLogValve, FastCommonAccessLogValve:
- http://people.apache.org/~kkolinko/patches/2009-07-02_tc55_AccessLogValve.patch
- http://people.apache.org/~kkolinko/patches/2009-07-01_tc55_FastCommonAccessLogValve.patch
- +1: kkolinko, markt, fhanik
- -1:
-
* Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=38743
Warn if the user tries to set an invalid property.
Port of http://svn.apache.org/viewvc?view=rev&revision=565464
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org