tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From kkoli...@apache.org
Subject 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/
Date Thu, 09 Jul 2009 23:38:00 GMT
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


Mime
View raw message