Bug 47158

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