Code log entry 084: After roughly 187 minutes of analyzing using the Java Visual VM, the cause of the non-terminating program was found. A lone ExecutorService was found to have never been shutdown(), and the internal thread pool had been keeping the JVM from terminating. A proper call to shutdown() was added to the offending class's stop() method, resolving the issue.
The problem I ran into above was a painful one to track down, but my intuition said the problem would make for a great FindBugs detector because it dealt with omission of code, which is easy for static analysis to detect and generally time-consuming for humans to detect.
What follows is how I designed a detector to hopefully make the struggles detailed in entry 084 obsolete. It is meant for readers who have not yet implemented a FindBugs detector, but are curious at what it entails and what a sample workflow might be.
FindBugs Detector Recipe
- Preparation time
- Coding time
- Derived from existing detector
- the purpose of a nice, easy introduction
When I implemented this detector, I contributed it to fb-contrib. Pedagogically, if you don't have to worry about messing up the actual FindBugs code and you want to focus more on the detector and less on all of FindBugs internals, so I recommend doing the same for your first detector.
This recipe assumes the following conditions:
- You have the source code for both FindBugs and fb-contrib downloaded and ready to build (I recommend Eclipse)
- You have read (or at least skimmed) Chris Grindstaff's high level overview of building a FindBugs detector at least once. Don't worry if you don't understand everything, we'll be covering a lot of the topics and, trust me, things get much clearer when you do them yourself.
- You've read the JavaDoc for Executors (the protagonist of our story) and perhaps browsed this sample code to get a feel for things.
- You have a JDK installed, and a reference to it in your path.
Setting up our TestBed
If you have ever done Test Driven Development, writing your test code first is not a new idea. For building FindBugs detectors, it is even more important because we need the source code to generate the Java bytecode that FindBugs scans. I haven't found a way to make FindBugs operate in a unit test scenario (i.e. run tests and watch a green bar as they pass), but I still write test cases with code that should and should not work.
Let's make a file, called HES_Sample.java, that will contain some buggy code. This will go in the samples folder when we are done with all the other test cases. I immediately replicated (and simplified) the case that caused me 3 hours of debugging to track down, as well as the fixed version:
For those of you thinking ahead, there are (at least) two more cases where ExecutorServices can be left hanging, or be misused, which I'll cover in a later blog post.
Finding some detector code to work with
Chris Grindstaff recommends finding a similar detector to base our new detector off, and I heartily agree, especially for the first, second or nth detector that you plan to write.
There is no magic way to find a similar enough detector, but I've had luck browsing through the fb-contrib messages.xml file until I find a message that seems familiar. I, by a stroke of luck, had been recently improving the message on the PMB_PossibleMemoryBloat detector, which seemed like a good start. It seems suitable because it detects collections that never get a call to remove(), clear() or similar methods (and thus always get larger but never get smaller), which is similar to what we want to do.
You clever people in the audience may have guessed that to make our HangingExecutors detector work, all we will need to do is copy most of this code into our detector and modify bloatableSigs from line 8 and decreasingMethods from line 15 to our ExecutorService specific signatures and we'd be basically done. You clever folk would be correct.
Visitor Pattern Interlude
Before we get into how to modify this code to suit our needs, let's go over the interesting parts, that is, the snipped out code and see exactly how this detection thing works. Before we get to it, I'd like to give a quick refresher on the Visitor pattern because the pattern is how detectors function -- FindBugs uses BCEL (JavaDocs) to walk through the compiled Java bytecode and creates "events" every time it runs into things, the beginning of a class or an individual bytecode, for instance. Our job is to override these listeners and keep track of the interesting bits to detect our bugs.
Generally, you will need the following four visitor methods to make a detector (There are many more than these four, but these will do most of the job). The order that these get called is:
That is, for every class, visitClassContext() is called [in this method we tend to look at fields), then for every method, we get a visitMethod() [usually to get the name or reset the OpcodeStack], then a visitCode() [similar to visitMethod()], and then for every opcode, a call to sawOpcode() is performed [the meat of most detectors goes here].
How does PossibleMemoryBloat work?
At a high level, PMB works in four steps:
- Find all Collection fields and put them in bloatableCandidates.
- While parsing through all methods, see if things ever get added to the Collection fields and add those to bloatableFields.
- While parsing through all methods, remove any fields that get a call to clear(), remove() or similar.
- Report bugs on all remaining Collection fields.
This seems straight forward enough. Because ExecutorServices are just dangerous if they don't get shutdown, our detector will omit step 2, but I'll still walk through the original PMB source code for that step.
The following source code handles step 1, the discovery of Collection-based fields:
The rest of the visitors are shown in the next code snippet. sawOpcode() does most of the work. A word of warning, some of the sawOpcode() methods in other detectors contain really long switch case methods and can be a bit daunting at first, but be strong! Carry on!
So that was all the code in the detector. Not trivial, but not so bad once you know a couple of FindBugs quirks, eh? Some of you may be wondering about the message that gets displayed. That involves three more files and is more of a bureaucratic step than a development step, therefore I'll kill two birds with one stone and explain the "paperwork" concepts while we make our own detector in the next section.
Building our custom "no shutdown() calls on an ExecutorService" detector
Making the message and other bookkeeping
Before we make our code, we'll need to make a message and tell FindBugs how to invoke our detector. That will involve adding entries to three files, etc/bugrank.txt, etc/findbugs.xml, and etc/messages.xml.
That new line of code [line 9 in the example] registers a new BugPattern called HE_EXECUTOR_NEVER_SHUTDOWN. A BugPattern is tied to a message and lives in exactly one Detector. A single detector can be responsible for one or more BugPatterns. We've already seen an example where one Detector was responsible for two BugPatterns.
What's the -2 for and why do all the others have 0? Well, that number modifies the severity of the bug that is reported. The severity follows the following formula:
Strictly speaking, adding the BugPattern to bugrank.txt is optional, because if there isn't an entry, the second modifier defaults to 0. PMB, as are a plurality of the bugs, is a "Correctness" bug. Our custom detector will also be in this category.
Now, to tell FindBugs that we have made a new detector and where to find it, we go to etc/findbugs.xml, and add two entries:
The final part, and arguably the most important part from a usability standpoint, is making an understandable message. This, of course, goes in etc/messages.xml, along with a description of the detector itself and one last BugCode. To explain the difference between the BugPattern message and the Detector description, see the following image:
And that wraps up the paperwork. Now, let's go finish writing the detector!
The modified code
Since we've already seen the meat of the detector, just in a slightly different form, I won't go into as granular of detail as I did earlier. The resulting code is below, with a few remarks on the changes.
Now, to compile the plugin, just run the build script that's a part of the fb-contrib source. It's an ant script, so it will take care of everything (compilation, dependencies, etc). The only thing you may have to do is to download yank.jar from Maven to act as the dependency manager and put it in your .ant/lib folder, which for me was in [HomeDirectory]/.ant/lib/ .
Finally, put your newly built fb-contrib.jar into the findbugs/plugin dir (for me, it was [eclipse install dir]/plugins/edu.umd.cs.findbugs.plugin.eclipse_184.108.40.20631122-15027/plugin/). Run Findbugs and marvel in your cleverness!
Congratulations! You've written your first FindBugs Detector. From here, the door only opens wider. Future posts will cover more advanced topics, like OpcodeStack gymnastics and looking at the bytecodes for inspiration to write a detector. Check back soon!