Bug Patterns In Java - The Split Cleaner - Online Article

Overview

When a program leaks or frees resources too early—resources such as memory, files, or database connections—a Split Cleaner may be the problem. We look at how the resource is managed as a potential cause.

One key feature of Java is that storage is automatically managed, saving the programmer from the bug-prone task of freeing memory after it has been used. Nevertheless, many programs still have to manipulate resources that must be explicitly freed after use. As is true in manual storage management, there are pitfalls that a programmer can encounter when managing resources in this way.

When managing a resource such as a file or a database connection, it is necessary to free the resource once you're done with it. Of course, on any given execution of the code, you want to obtain and free the resource exactly once. There are a couple of ways you could go about doing this:

  • Obtain and free the resource in the same method. This way, you can guarantee that each time the resource is obtained, it is also freed.

  • Trace through each possible execution path of the code. This way, you have to check that the resource is eventually freed in each case.

The second option is problematic. Because your code base will inevitably grow, another programmer who is not familiar with your code may add another execution path in which the resource is not freed, resulting in resource leakage.

I call bugs that fit this pattern Split Cleaners because the clean up code for the resource is split along the various possible execution paths. Because the clean up code along each path is likely to be identical, most Split Cleaners are also examples of Rogue Tiles.

We can summarize this bug pattern as follows:

  • Pattern: Split Cleaner.
  • Symptoms: A program that improperly manages resources, either by leaking them or by freeing them too early.
  • Cause: Some execution paths of the program do not free the resource exactly one time as they should.
  • Cures and Preventions: Move the code that handles clean up into the same method that obtains the resource.

About This Bug Pattern

As we said earlier, since the clean up code along each path is likely to be identical, most Split Cleaners are also examples of Rogue Tiles.

For example, suppose you are using JDBC to manipulate a table of employee names. Many of the operations you want to perform involve walking over this table and computing over the contained data. One thing you may end up doing is printing out the first names of all your employees, like so:

Code That Walks over a Table of Employees

import java.sql.*;
public class Example
{
public static void main(String[] args)
{
String url = "your database url";
try
{
Connection con = DriverManager.getConnection(url);
new TablePrinter(con, "Names").walk();
}
catch (SQLException e)
{
throw new RuntimeException(e.toString());
}
}
}
abstract class TableWalker
{
Connection con;
String tableName;
public TableWalker(Connection _con, String _tableName)
{
this.con = _con;
this.tableName = _tableName;
}
public void walk() throws SQLException
{
String queryString =("SELECT * FROM "+ tableName);
Statement stmt = con.createStatement();
ResultSet rs = stmt.executeQuery(queryString);
while (rs.next())
{
execute(rs);
}
con.close();
}
public abstract void execute(ResultSet rs) throws SQLException;
}
class TablePrinter extends TableWalker
{
public TablePrinter(Connection _con, String _tableName)
{
super(_con, _tableName);
}
public void execute(ResultSet rs) throws SQLException
{
String s = rs.getString("FIRST_NAME");
System.out.println(s);
}
}

First, a short digression. Notice that we've pulled out the code to handle walking over the table into an abstract Walker class, allowing for new subclasses to easily walk over the rows of a table. Although it is often a waste of time to try to anticipate and code for all the ways in which a program will be extended, let's suppose that in this case there is absolutely no doubt whatsoever that exactly this kind of extension will be made to the code.

Tip

It is often a waste of time to try to anticipate and code for all the ways in which a program will be extended. Instead, try to find the simplest design that will solve the problem at hand, and cover your code with unit tests.When you need to extend or modify the program's functionality, your unit tests will allow you to do so with confidence.

The Symptoms

Now, notice that the database connection is passed into the constructor of the TableWalker. Once it is done walking over the table, it closes the connection. So, in this case, we've used the second strategy for cleaning up the connection: we've attempted to close the connection separately along each execution path.

Let's suppose that in the context of our system it makes sense to close the connection after a single walk over the data (for example, perhaps this code is intended to be called from the command prompt). Even in that case, we haven't caught every possible execution path: if an SQLException is thrown, the program may abort before ever closing the connection.

Because SQLExceptions are not so common in mature code, this bug may not demonstrate any symptoms for a long time, perhaps not until the original developer is no longer available. Naturally, this will make things harder to diagnose when the symptoms do appear.

But if the code is extended, there are ways in which symptoms might appear much more rapidly. For instance, let's suppose that after the original code was written, it became clear that the bulk of the phone numbers on file were out of date. So management decides that all employee phone numbers should be replaced with "411." To perform this update, a new TableWalker could be written, as follows:

Code Walker to Update Old Data

class TableFixer extends TableWalker
{
public TableFixer(Connection _con, String _tableName)
{
super(_con, _tableName);
}
public void execute(ResultSet rs) throws SQLException
{
String s = rs.getString("FIRST_NAME");
String updateString = "UPDATE "+ tableName +
"SET PHONE_NUMBER = "+ "411" +
"WHERE FIRST_NAME LIKE '"+ s + "'";
Statement stmt = con.createStatement();
stmt.executeUpdate(updateString);
}
}

Because TableFixer also extends TableWalker, calling walk() on an instance of this class will close the connection to the database, just like TablePrinter. If a program were to try to make instances of both walkers with the same connection, it would find that the connection was closed as soon as the first walker finished its walk.

It would be easy for an incoming programmer to make this mistake, especially if the invariant—that only one walker could be constructed—wasn't documented or tested.

The Cause

Some execution paths of the program do not free the resource exactly one time as they should.

Cures and Preventions

When you find an execution path that doesn't include the appropriate clean up code, you might be tempted to simply add it to that path. For example, you could wrap the body of the walk() method in a try block, and put in a finally clause to guarantee that the connection will be closed. But such a solution would be a bad fix.

There is really no reason why our TableWalkers should need to worry about closing the connection at all. But even if each TableWalker did try to close the connection, we would run into the second way in which this bug pattern can manifest itself: when we want to run multiple walkers, too many attempts are made to close the connection.

Worse still, if we were to put in two calls to con.close() (one in the try block and one in the catch block, as opposed to a single call in a finally clause), we would have introduced a Rogue Tile into the code. If many Rogue Tiles were added, it would become quite difficult to ever refactor the code successfully. Some of the Rogue Tiles might handle rare execution paths that may not occur during testing.

A much better solution would be to refactor the code to use the first approach to managing such resources: put the code for obtaining and releasing a resource in the same method. In their excellent book, The Pragmatic Programmer, Andrew Hunt and Dave Thomas advocate this idea with the phrase "Finish What You Started."

Each method should be responsible for cleaning up the resources that it acquires. In our example, this would involve moving the call to con.close() into the main method of class Example, as follows:

Code Refactored So That Resources Are Obtained and Released in the Same Method

class Example2
{
public static void main(String[] args)
{
String url = "your database url";
// con must be declared and initialized here, so as to be in
// the scope of both the try and finally clauses.
Connection con = null;
try
{
con = DriverManager.getConnection(url, "Fernanda", "J8");
new TablePrinter(con, "Names").walk();
}
catch (SQLException e)
{
throw new RuntimeException(e.toString());
}
finally
{
try
{
con.close();
}
catch (Exception e)
{
throw new RuntimeException(e.toString());
}
}
}
}

Here, the call to con.close() is in a finally clause of the same try block that created the connection; thus, an execution path in which it's not called is impossible.

What We've Learned

In this article on the Split Cleaner bug pattern we've learned the following:

  • With Split Cleaners, the clean up code for a resource is split along the various possible execution paths.
  • Most Split Cleaners are also examples of Rogue Tiles because the clean up code along each path is likely to be identical.
  • One way to guarantee that you obtain and free a resource exactly once is to obtain and free it in the same method.
  • Another (bad) way is to trace through each possible execution path of the code to ensure that the resource is eventually freed in each instance. This will cause maintenance nightmares when a programmer unfamiliar with your code adds another execution path in which the resource is not freed.
  • When you find an execution path that doesn't include the appropriate clean up code, you might be tempted to simply add it to that path. That would be a bad fix.
  • Each method should be responsible for cleaning up resources that it acquires.
  • Extending code can make Split Cleaners appear more rapidly.
  • It is often a waste of time to try to anticipate and code for all the ways in which a program will be extended.

About the Author:

No further information.




Comments

No comment yet. Be the first to post a comment.