"Double-Checked Locking" pattern is heavily used in this class, but the usage of it in AccessLogValve has some issues of race condition judging by this article: http://www.cs.umd.edu/~pugh/java/memoryModel/DoubleCheckedLocking.html Based by this article, I think these variables in this class should be declared as volatile to get rid of race condition: private volatile long currentMillis; private volatile Date currentDate;
I agree with your analysis for currentMIllis. I disagree with your analysis for currentDate. I think we can remove all the places this is updated except getDate() and still have the same functionality. I also think dateStamp needs to be volatile. Patch for the above has been committed to trunk and proposed for 6.0.x.
The patch has been applied to 6.0.x and will be in 6.0.21 onwards. Filip noted in his review comments that there is also a thread safety issue around log file rotation. This still needs to be fixed.
The remaining threading issues have been fixed and will also be in 6.0.21