Will refactored methods in a Java-Struts web application cause contention?

Question

I'm working with a company that produces a web application using struts with Java. I have been refactoring a lot of the code recently to tidy up the system. One technique I've been using is to move closing statements for streams, in finally blocks, of methods to a utility class. The utility class is static and has close methods for various types of streams. By doing this refactoring I save 5 lines of code every time a stream needs to be closed which has reduced code in some classes by 600 lines making it very favourable. However I'm worried by doing this that it may be possible to cause contention on those method if put into production.

I've only come across the concept of contention in multithreaded programming before which is why I'm not sure if this will cause problems in these instances.

Is this possible? or simply my misunderstanding of contention, static methods, web applications etc?

Thanks in advance, Alexei Blue.

Answer 1

Unless the static methods operate solely on their parameters and you don't try to concurrently close the same stream multiple times you should be fine.

Methods in general can be executed simultaneously. Synchronzation will always work on objects and thus contention should not occur if the objects you use in those methods are completely distinct.

Answer 2

It is best practice to close all the open file handler within the same scope. For example, when a file outputstream is open in a method, then it should also be closed with in that method. This also prevent most of concurrency problem (but not always). However, there are many exception for not doing so but tracking and closing all file handler would be more difficult in this case.

FYI, there exists a closing method which does exactly what you describe in Apache Common IO, IOUtils. It is a popular library which you may consider using.

Answer 3

This is a common practice, for instance look at the Closeables class in Guava. By the way, you should only need one method to be able to close virtually anything in Java:

public static void closeQuietly(Closeable closeable) {
  if (closeable != null) {
    try {
      closeable.close();
    } catch (RuntimeException re) {
      // Do something or ignore...
    }
  }
}

As all the Streams and Readers implement the Closeable interface, this one method should suffice for all your needs.

In general this will not cause contention, unless you try to read from the stream in say Thread-A, where Thread-B is closing the stream. But if that were the case, then you'd have this contention with or without this refactoring. Solely this refactoring will not introduce any additional contention.