Strange code in tomcat's cache source code

user_16116287 0
Viewed 37

I am reading the source code of tomcat, and I notice something strange in org.apache.catalina.webresources.CachedResource:

@Override
public long getLastModified() {
    Long cachedLastModified = this.cachedLastModified;
    if (cachedLastModified == null) {
        cachedLastModified =
                Long.valueOf(webResource.getLastModified());
        this.cachedLastModified = cachedLastModified;
    }
    return cachedLastModified.longValue();
}

What if I replace it with:

@Override
public long getLastModified() {
    if (this.cachedLastModified == null) {
        this.cachedLastModified =
                Long.valueOf(webResource.getLastModified());
    }
    return this.cachedLastModified.longValue();
}

There are several functions like this. I don't understand it is written like this.

tomcat
1 Answers

It is almost 10 years since I wrote that code.

It was written that way for thread-safety reasons. The original code set this.cachedLastModified (and the obvious other fields) to null when the cache entry expired. The getter was written that way to avoid NPEs.

A subsequent refactoring changed how cache expiry was implemented which removed the thread-safety issues but the associated protection wasn't removed from the getters.

The getters could be refactored to remove the now unnecessary protection and simplify the code.

You can see the original code and how the cache expiry implementation changed in the svn log.

Generally, if you see a copy of a field or fields being taken at the start of a method in the Tomcat code base the reason will nearly always be to address a thread safety issue.

user_12990050
Related