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.
CodePudding user response:
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.