Category Archives: Best Practices

Proper use of Log4J for logging exceptions

I see a lot of developers at my organization who are using Log4J incorrectly when it comes to logging exceptions.

Invariably, the desire is to log the error and get a meaningful message from the exception that is thrown.

Consider the following code:

Logger log = Logger.getLogger(ClassName.class);
try
{
    //Some code here...
}
    catch(SomeException e)
{
    log.error("Some error has occurred");
}

This block of code sets up the context for our example.¬† We’ll just worry about the log.error statement from here on.

The most common errors I see in logging exceptions are:


log.error(e);

and


log.error("An error has occurred: " + e.getMessage());

Unfortunately, neither of these calls will generate the output that the developer wants.

Ultimately, the developer wants to get a meaningful message that will help them identify and debug the problem.
Log4J does provide a simple way to generate a full stack trace for an exception in your logs:

log.error(String msg, Throwable e)

So you could modify the incorrect examples I listed above to do what you really want:

log.error("Some Error Message", e);

This code will generate an entry in the log that contains your error message AND a full stack trace for the exception that was caught.

Everyone needs a code review

I’ve been writing code for a long time and now that I’m an Architect at my company, people look at me funny when I ask them to review my code. Many folks think that I don’t need code reviews.

My feeling on the matter is that everyone needs a code review. Code reviews don’t have to be long formal meetings with code printouts and red pens to mark all the issues. A simple peer review before checking your changes into the Source Control System may be all that is needed in many cases.

Here’s a recent example of a quick change I made to code and pushed through to production. It was such a small change that I didn’t think to get it reviewed. Now, I wish I had as we’re having to make an off-cycle release of the code to fix the bug that I introduced. Continue reading Everyone needs a code review

How Not to Initialize Log4J in an Application

We had a thread leak on our production servers recently that caused downtime on multiple applications.  The culprit turned out to be an application that configured Log4J incorrectly.

How can Log4J bring down an application server?¬† I don’t think this one was Log4J’s fault so much as a developer who initialized it incorrectly.

As you may know, Log4J has a nice feature called ConfigureAndWatch which allows you to specify a configuration file and an interval.  Log4J will check the configuration file for updates on the specified interval and will update the configuration as needed.   This is a very useful feature as it allows us to configure logging on the fly on our production servers.  Thus, when an app begins to misbehave, we can enable more detailed logging without having to restart the application (which would reset the app and potentially make the error disappear for awhile).

The danger with ConfigureAndWatch is that by its very nature, it must spawn a new thread when it is called.¬† This thread sleeps for a period of time, then wakes up to check the config file.¬† This works well if you initialize Log4J properly as it is only called once when the application starts up.¬† Unfortunately, a developer will occasionally embed a call to ConfigureAndWatch in the main execution path of the application.¬† In the case of a web application, this results in a new thread being spawned each time the application gets a hit.¬† The threads never die, they just keep going.¬† Because they sleep most of the time, they don’t show up on system load reports and such until the system eventually cannot allocate any more threads.

On our LINUX systems, java threads count as OS processes.  As the OS reached its process limit, the application crashed as did the container, and everything else that was owned by the same user.

I caught wind of the thread leak and someone involved mentioned that all the threads were running the FileWatchDog class which is the class used by Log4J to monitor files.  We were able to identify the offending app and issue a fix, but this should have never happened.

In general, developers should follow the following rules when developing application code:

  • Shared libraries should not run any kind of Log4J initialization code.¬† This should be left to the applications that will use the library.
  • If you don’t need the ability to change your logging configuration at run-time, you should allow Log4J to configure itself by simply including a log4j.properties or log4j.xml file in the application classpath.
  • Never call ConfigureAndWatch in a way where it can be executed more than once during an application’s life.

How to use DataSources in a Web Application on Oracle OAS10g

There are many how-to documents on using DataSources in web applications. I’m adding yet another, mainly because I want one in a location that I can remember when I need to point someone at it. Additionally, there aren’t a ton of documents on how to set all this up for OAS10g application server.

This document will outline the proper way to reference a DataSource on OAS and how to set up the application-specific configuration file with a default JNDI mapping for the logical DataSource.
Continue reading How to use DataSources in a Web Application on Oracle OAS10g

Managing JDBC Database Connectivity

The single largest source of bugs in my shop lies around managing database connectivity in applications.

Now, my shop is still in the dark ages in that we typically don’t use any kind of persistence framework. Instead, we code our SQL by hand into our classes.

When working with a database in this way, you’ll need to manage several different objects, making sure to properly close each of them in turn or you’ll end up with resource leaks. This is especially true if you are using a connection pool to manage your database connections.

The objects that you need to manage are:

  • Connection
  • Statement
  • ResultSet

You’ll open each of these in turn, and each must be closed out when you are finished with it. Each of these objects has a close() method on it for just this purpose.

It’s common for me to come across the following code during code reviews:

DataSource ds = (Some DataSource here)
try{
   Connection c = ds.getConnection();
   Statement s = c.createStatement();
   ResultSet r = s.executeQuery("SELECT * FROM DUAL");

//Code to process the query results

   r.close();
   s.close();
   c.close();
}
catch(SQLException e)
{
   // Log the error
}

At first glance, this may look correct. However, we have to think about what happens if an exception is thrown before one or all of the database objects are closed.

A better solution is to close out the database objects in a finally block which is guaranteed to run:

DataSource ds = (Some DataSource here)
try{
   Connection c = ds.getConnection();
   Statement s = c.createStatement();
   ResultSet r = s.executeQuery("SELECT * FROM DUAL");

//Code to process the query results
}
catch(SQLException e)
{
   // Log the error
}
finally
{
   r.close();
   s.close();
   c.close();
}

Of course, this is still problematic as any of the close methods can also throw a SQLException. We can get a little crazy and nest some additional try / catch blocks inside our finally block to be certain that we close out everything:

...
finally
{
   try
   {
      r.close();
   }
   catch(SQLException e)
   {
      // Log Exception
   }
   //use a finally here in case some other RunTime exception
   //is thrown in the previous close statement
   finally   {
      try
      {
         s.close();
      }
      catch(SQLException e)
      {
         // Log Exception
      }
      //use a finally here in case some other RunTime exception
      //is thrown in the previous close statement
      finally
      {
         try
         {
            c.close();
         }
         catch(SQLException e)
         {
            // Log Exception
         }
      }
   }
}

The above will ensure that all database objects are closed.

Until my shop moves to embrace a persistence framework or ORDB mapping solution such as TopLink or Hibernate, I am stuck using the above method.

One alternative that I’ve begun to endorse is to use the Commons-DBUtils library from the Jakarta Commons project. This API allows you to pass in a DataSource, a Query, and a Query Processor. You never touch connections, statements, or result sets. As such, the API can ensure that all objects are closed correctly. This removes some of the tedium of doing direct JDBC calls from within your code and helps to avoid resource leaks by ensuring that all DB resources are properly managed.