Bug 47158 - I think AccessLogValve has race condition problem
Summary: I think AccessLogValve has race condition problem
Status: RESOLVED FIXED
Alias: None
Product: Tomcat 6
Classification: Unclassified
Component: Catalina (show other bugs)
Version: unspecified
Hardware: PC Windows XP
: P2 normal (vote)
Target Milestone: default
Assignee: Tomcat Developers Mailing List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-05-05 15:12 UTC by Xie Xiaodong
Modified: 2009-11-02 13:58 UTC (History)
0 users



Attachments

Note You need to log in before you can comment on or make changes to this bug.
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