Breaking code by switching input streams 2015-11-18

This summer I was refactoring code for reading class files in a Java compiler. The code used a custom input stream, and the implementation did essentially what BufferedInputStream from the Java standard library did. I decided it was redundant to use a custom input stream, and I should be able to replace it by the standard library class. However, doing this caused many errors in the regression test suite for the compiler.

This seemed odd. Replacing one input stream by another should have no effect on the code using the stream, right? After taking a closer look at the custom input stream, it really seemed like it followed the contract of the InputStream documentation, so why didn’t it work?

The first step to solving a compiler bug is usually to create a minimal test case: a minimal input program that exposes the same compile error. I could not do that in this case because the inputs that were crashing the compiler were class files – a binary file format that is difficult to edit by hand.

Another way to start debugging a problem is to try to get closer to the root cause – even though you might not know exactly where the root cause is located you can often make an educated guess. I assumed that whatever the root cause was, it was causing the bytecode reader to output an incorrect sequence of bytes to the class file parser at some point. Figuring out when that divergence happened should help to locate the cause. I put some printouts in the class file parser to log the bytes it saw coming from the bytecode reader. I then ran the compiler on a failing test and diffed the output between the working compiler and the faulty version, it was then easy to see that the outputs diverged just a few hundred bytes into one particular class file.

When I knew where in the class file the outputs diverged I could run the compiler in a debugger and set a breakpoint right before it reached that point. I single-stepped through to see what was happening call-by-call, and I saw that the problems started appearing right after a call to InputStream.skip(). I looked again at the documentation and saw that skip() is not guaranteed to always skip ahead the specified number of bytes. The code in the bytecode reader seemed to assume that though, because there was only one call to skip(). The correct way to use skip() is to call it repeatedly, using the return value which indicates how many bytes were actually skipped:

int remaining = bytesToSkip;
while (remaining > 0) {
  remaining -= in.skip(remaining);
}

The original input stream, the custom-made one, always ensured to skip the specified number of bytes, so when it was replaced by BufferedInputStream, which does not always skip the given number of bytes, things went wrong in the code that assumed all input streams worked the way the custom-made one did!

This is a valuable lesson in programming: even if you use one class that follows a specification by another that follows the same specification, you might end up breaking your code! This was not the first time I’ve had this kind of problem, and I suspect that it is a common and especially difficult to diagnose error in programming. A recent and wide-spread example of this type of problem is HashSet ordering in Java 8, where the iteration order has changed since the Java 7 HashSet implementation. This is all within the specification for a HashSet, but many uses inadvertently depended on the order being deterministic and switching to Java 8 then breaks the code.

Categories: Uncategorized

Leave a Reply

Your email address will not be published. Required fields are marked *