The Danger of Static Variables in Multithreaded Environments: An Example

I fixed a very subtle bug the other day that illustrates the dangers of using static variables in multithreaded environments, such as multi-user web applications. I had implemented a Singleton class in a non-multithreaded desktop application many months ago that contained a static map (actually Java's HashMap) of database (DB) connections. The map was used to cache opened connections to different DBs so they could be reused by different parts of the application. When some part of the application needed a connection, my singleton was used to get one such that if an opened connection already existed it was returned; otherwise a new connection was opened, added to the map, and returned (a trivial connection pool if you will). A little while ago I needed to update this class for use in a multithreaded server for a web application. So, I replaced the singleton with a class that could be instantiated by each user (i.e. thread) using the default constructor. When doing the changes, I missed removing the static storage class from the declaration of the map instance and ended up with a class something like this (only shows what is necessary):

class MyConnectionManager
{
private static final Map<String,Connection> _map =
  new HashMap<String,Connection>();

public Connection getConnection(String db)
{
  if (!_map.containsKey(db))
  _map.put(db, makeNewConnection(db));
  return _map.get(db);
}
....
}

Even though I did have unique instances of the manager class for each user/thread, all instances used the same Map. There are at least two threading issues here. The first is that two or more different users could use the same connection. Even if the Connection class is thread safe, we still have a problem if thread A closes the connection while thread B is using it. The second issue is that HashMap is not thread safe. One fix to all thread safety issues in this case is simply to remove the static storage class from the declaration of the Map instance.