Recently, we found a bug in our Solr related project. Basically the problem is that:
1. we create a SolrQueryRequest but forget to close it when it is no longer needed.
2. We call SolrCore.getSearcher(), but forgot to decref the RefCounted.
This will cause one SolrIndexSearcher(thus its various kinds of cache) left unclosed every time we run a commit, at last will cause OutOfMemoryError.
RefCounted refCounted = req.getCore().getSearcher();
Solr RefCounted: Don't forget to close SolrQueryRequest or decref solrCore.getSearcher
After realized this bug, we searched our code to fix all this kind of bugs. But how we can prevent this same kind of error happens again in the future.
This make me to think to write a findbugs plugin to detect and report this kind of bug. The following is how to do this.
Test-Driven: How to test our findbugs detector
When we learn code or write code that we are not familiar, the best way is to write code, test it, make change and test it again.
So it's important to know how to write test code to test our detector.
Fortunately, there is project: test-driven-detectors4findbugs in github which lets us write test code for our detector easily.
Test Driven Detectors 4 FindBugsIt is for FindBugs 1.3.9, we need make some small change to make it work with FindBugs 2.x. I forked the project and make it work with FindBugs 2.x, and put it at Github.
The following is the test code for NotClosedRequestDetector:
The best way is to learn from source code of findbugs(and its findbugsTestCases) and fb-contrib projects.
Check where we forgot to close SolrQueryRequest
The idea is simple, if a method initialize a SolrQueryRequest, we increment solrQueryInitCount by one, if it call SolrQueryRequest.close(), we decrease solrQueryInitCount by one.
If solrQueryInitCount is not zero at the end of method, we report a bug.
This implementation is not perfect, but anyway it works. In future we can improve it, also we can add one more detector to make sure the SolrQueryRequest.close and refCounted.decref() is called in a finally block.
The code is like below, the complete source/test code, findbugs.xml and messages.xml can be found at Github.
Similarly, I create another class UnDecrefRefCountedSearcherDetector to detect where we forget to decref RefCounted, source code can be found at Github.
How to deploy it
We can directly put the built jar into eclipse\plugins\edu.umd.cs.findbugs.plugin.eclipse_***\plugin, or add the jar in Preferences -> FindBugs -> "Plugins and misc. Settings" tab.
It works
The following picture shows how it looks like after deploy my findbugs plugin.
Resources:
1. we create a SolrQueryRequest but forget to close it when it is no longer needed.
2. We call SolrCore.getSearcher(), but forgot to decref the RefCounted
This will cause one SolrIndexSearcher(thus its various kinds of cache) left unclosed every time we run a commit, at last will cause OutOfMemoryError.
RefCounted
Solr RefCounted: Don't forget to close SolrQueryRequest or decref solrCore.getSearcher
After realized this bug, we searched our code to fix all this kind of bugs. But how we can prevent this same kind of error happens again in the future.
This make me to think to write a findbugs plugin to detect and report this kind of bug. The following is how to do this.
Test-Driven: How to test our findbugs detector
So it's important to know how to write test code to test our detector.
Fortunately, there is project: test-driven-detectors4findbugs in github which lets us write test code for our detector easily.
Test Driven Detectors 4 FindBugsIt is for FindBugs 1.3.9, we need make some small change to make it work with FindBugs 2.x. I forked the project and make it work with FindBugs 2.x, and put it at Github.
The following is the test code for NotClosedRequestDetector:
public class NotClosedRequestDetectorTester {
@Test public void demoDecrefRefCountedSearcher() throws Exception {
BugReporter bugReporter = DetectorAssert.bugReporterForTesting();
NotClosedRequestDetector detector = new NotClosedRequestDetector(
bugReporter);
DetectorAssert.assertNoBugsReported(DemoClosedSolrRequest.class,
detector, bugReporter);
}
@Test public void demoUnClosedSolrRequest() throws Exception {
BugReporter bugReporter = DetectorAssert.bugReporterForTesting();
NotClosedRequestDetector detector = new NotClosedRequestDetector(
bugReporter);
DetectorAssert.assertBugReported(DemoUnClosedSolrRequest.class,
detector, bugReporter);
printBugReporter(detector, bugReporter);
assertFindbug(bugReporter,
NotClosedRequestDetector.NOT_CLOSE_SOLR_REQUEST);
}
}
Learn how to write custom detectorThe best way is to learn from source code of findbugs(and its findbugsTestCases) and fb-contrib projects.
Check where we forgot to close SolrQueryRequest
The idea is simple, if a method initialize a SolrQueryRequest, we increment solrQueryInitCount by one, if it call SolrQueryRequest.close(), we decrease solrQueryInitCount by one.
If solrQueryInitCount is not zero at the end of method, we report a bug.
This implementation is not perfect, but anyway it works. In future we can improve it, also we can add one more detector to make sure the SolrQueryRequest.close and refCounted.decref() is called in a finally block.
The code is like below, the complete source/test code, findbugs.xml and messages.xml can be found at Github.
public class NotClosedRequestDetector extends BytecodeScanningDetector {
public static final String NOT_CLOSE_SOLR_REQUEST = "NOT_CLOSE_SOLR_REQUEST";
private int solrQueryInitCount = 0;
private List<Integer> solrQueryInitPCList = new ArrayList<Integer>();
private final BugReporter bugReporter;
private final BugAccumulator bugAccumulator;
public NotClosedRequestDetector(BugReporter bugReporter) {
this.bugReporter = bugReporter;
bugAccumulator = new BugAccumulator(bugReporter);
}
@Override public void visit(Code obj) {
solrQueryInitCount = 0;
solrQueryInitPCList.clear();
super.visit(obj);
if (solrQueryInitCount > 0) {
for (Integer pc : solrQueryInitPCList) {
bugAccumulator.accumulateBug(new BugInstance(this,
NOT_CLOSE_SOLR_REQUEST, HIGH_PRIORITY)
.addClassAndMethod(this), SourceLineAnnotation
.fromVisitedInstruction(getClassContext(), this, pc));
}
bugAccumulator.reportAccumulatedBugs();
}
}
@Override public void sawOpcode(int seen) {
try {
if (seen == INVOKESPECIAL) {
boolean isSolrRequest = isSolrRequest();
if (isSolrRequest) {
if (getNameConstantOperand().equals("<init>")) {
solrQueryInitCount++;
solrQueryInitPCList.add(getPC());
}
}
} else if (seen == INVOKEVIRTUAL || seen == INVOKEINTERFACE
|| seen == INVOKEINTERFACE_QUICK) {
boolean isSolrRequest = isSolrRequest();
if (isSolrRequest) {
if (getNameConstantOperand().equals("close")
&& getSigConstantOperand().equals("()V")) {
solrQueryInitCount--;
}
}
}
} catch (ClassNotFoundException cnfe) {
bugReporter.reportMissingClass(cnfe);
}
}
private boolean isSolrRequest() throws ClassNotFoundException {
String className = getClassConstantOperand();
if (className == null)
return false;
JavaClass classClass = Repository.lookupClass(className);
JavaClass requestClass = Repository
.lookupClass("org.apache.solr.request.SolrQueryRequest");
return classClass.instanceOf(requestClass);
}
}
Check where we forgot to decref RefCountedSimilarly, I create another class UnDecrefRefCountedSearcherDetector to detect where we forget to decref RefCounted
How to deploy it
It works
