Bug 31018 - Race condition in SystemLogHandler.java
Summary: Race condition in SystemLogHandler.java
Alias: None
Product: Tomcat 5
Classification: Unclassified
Component: Unknown (show other bugs)
Version: 5.0.28
Hardware: Other All
: P3 normal (vote)
Target Milestone: ---
Assignee: Tomcat Developers Mailing List
Depends on:
Reported: 2004-09-02 15:55 UTC by Ron Gomes
Modified: 2004-11-16 19:05 UTC (History)
0 users


Note You need to log in before you can comment on or make changes to this bug.
Description Ron Gomes 2004-09-02 15:55:31 UTC
The org.apache.tomcat.util.log.SystemLogHandler class (source code in
jakarta-tomcat-connectors) handles, among other things, capturing of
System.out and System.err so that they can be redirected to an
application's log file.  It makes use of another class (CaptureLog) to
hold the captured data, and maintains a stack of unused CaptureLog
instances to avoid creating new ones where possible.
This "reuse" stack is global to the class and it looks like the use of
it is not thread-safe.  The startCapture() method acquires a
CaptureLog() this way:
    if (!reuse.isEmpty()) {
        log = (CaptureLog)reuse.pop();
    } else {
        log = new CaptureLog();
There's a race between the call to isEmpty() and the call to pop().
We've been able to reliably elicit a java.util.EmptyStackException at
this point with an application under heavy load.
Replacing the above code with
    synchronized (reuse) {
        log = reuse.isEmpty() ? new CaptureLog() : (CaptureLog)reuse.pop();
eliminates the problem (with no effect on performance that we could
An alternative approach might be to catch the (rare)
EmptyStackException and treat it as equivalent to the isEmpty==true
case, but this seems cleaner.
There would seem to be no need to synchronize the code that pushes
old CaptureLog instances onto the reuse stack since java.util.Stack
is already synchronized.
Comment 1 Yoav Shapira 2004-09-02 18:52:57 UTC
Fixed, thanks.