Refactoring: Resource management

Set them free, always

Never keeps the keys of a resource

Resources (connections, files, memory) are typically non-shared objects. Two clients cannot write in the same file or memory area at the same time or perform concurrent transactions on the same connection. This is a matter of consistency, as concurrent writes may result in lost updates, and even concurrent write vs read could lead to a loss of isolation. In all cases, you would loose atomicity as well.

To avoid those problems resources are typically acquired (more or less exclusively depending on your read or write intent), then released: you extract a connection from a pool, you lock a file for writing, you allocate memory, and so on.

The implication of this is that you must make sure that resources will always be cleaned up. That may sound as an easy end-of-usage statement… when all goes well.

Error management

Yes, you must ensure that resources are released even when an error occurs. Hopefully most languages (including JavaScript, Java, Python) now support finally blocks that ease such “in any case” handling.

However, even when using it, here is what (too) many developers would write to warrant resource release:

let resource = null
try {
  resource = Resource.open()
  resource.use()
} catch (err) {
  console.error(err)  
} finally {
  if (resource != null) {    // open() may have failed
    resource.close()
  }
}

(Error handling is simplified here as a console message, but you’d want to implement it better by re-throwing a more insightful error or even not catching the original one, so that the caller will be able handle the failure at its own level)

This code is incorrect, for two reasons:

• it is needlessly complex and obscure ;
• it contains a fatal flaw.

The flaw

“But it works” may you say, as you have used it many times without a problem. Yes, it works in most cases, but not all. Actually, as Michael T. Nygard related in his excellent book Release It!:

[That code] grounded an entire globe-spanning, multibillion dollar airline company with its hundreds of aircraft and tens of thousands of employees.

Why? Because resource.close() can raise an error too. Some developers caring about this may actually make it even more cumbersome:

let resource = null
try {
  resource = Resource.open()
  resource.use()
} catch (err) {
  console.error(err)
} finally {
  if (resource != null) {
    try {
      resource.close()
    } catch (closeError) {
      console.error('Could not close', closeError) 
    }
  }
}

I’ve seen this code too often, especially as (now old) Java code*, and this is why I’m using it as a code test for technical interviews.

The clean way

As often, the cause of problems here is a mix of responsibilities. You should not do 2 things at a time:

1. acquiring the resource and using it in the same try {} block. Instead you should try to acquire the resource and, if this succeeds, try to use it.
2. handling errors and releasing resource at the same level and in the same finally {} block.

Instead you should separate the error handling contract from the resource release contract:

try {                             // Error handling start
  var resource = Resource.open()
  try {                           // Resource handling start
    resource.use()
  } finally {
    resource.close()              // We know resource is there
  }
} catch (err) {
  console.error(err)              // Any error handled here
}

Et voilà! Here we clearly simplified handling by separating the two contracts:

• error handling is most enclosing one, as anything can cause an error ;
• resource management is enclosed in error handling, so that it doesn’t need to care about errors. It only cares about using an already-acquired resource (if the acquisition fails, the code will interrupt to immediately execute the outer catch block) and so the closing block doesn’t have to wonder if the resource can be null at this time.

*Because a lot of developers had a hard time managing this properly, Java 7 has since then introduced a new “try with resource” (a.k.a. Automatic Resource Block Management) syntax that helps warrant auto-closing of any resource that correctly implement the AutoCloseable interface.