|Summary:||I think AccessLogValve has race condition problem|
|Product:||Tomcat 6||Reporter:||Xie Xiaodong <xxd82329>|
|Component:||Catalina||Assignee:||Tomcat Developers Mailing List <dev>|
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