Bug 47158

Summary: I think AccessLogValve has race condition problem
Product: Tomcat 6 Reporter: Xie Xiaodong <xxd82329>
Component: CatalinaAssignee: Tomcat Developers Mailing List <dev>
Severity: normal    
Priority: P2    
Version: unspecified   
Target Milestone: default   
Hardware: PC   
OS: Windows XP   

Description Xie Xiaodong 2009-05-05 15:12:45 UTC
"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;
Comment 1 Mark Thomas 2009-06-04 08:39:30 UTC
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.
Comment 2 Mark Thomas 2009-06-12 15:38:41 UTC
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.
Comment 3 Mark Thomas 2009-11-02 13:58:48 UTC
The remaining threading issues have been fixed and will also be in 6.0.21