Uploaded image for project: 'Log4j 2'
  1. Log4j 2
  2. LOG4J2-1553

AbstractManager should implement AutoCloseable

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: Appenders
    • Labels:
      None

      Description

      The class AbstractManager should implement java.lang.AutoCloseable.

      A manager holds on to resources that must be cleaned up. Typically these resources are allocated on instance creation and freed by calling the manager's release() method.

      For AbstractManager to implement java.lang.AutoCloseable, release() will re-implemented as close() and deprecated.

      There are several benefits to this change:

      • Make it obvious and formal that this kind of object must be properly managed.
      • A reader or cloner of the code will know that there is a pattern to follow.
      • This will make our tests cleaner, smaller and hopefully less error-prone.

      Possible disadvantages:

      • Coders may think that managers can be used like resources like a java.io.File.
      • Some AutoCloseable implementations are short-lived, this one not so much, so we will add Javadoc for this effect and to note the benefit for unit tests.

      Our current unit test code follows this pattern:

      SomeManager mgr = new SomeManager(lots, of, params);
      try {
          // test code
      } finally {
          mgr.close();
      }
      

      Sometimes, we also have:

      SomeManager mgr = new SomeManager(lots, of, params);
      try {
          // test code
      } finally {
          if (mgr != null) {
            mgr.close();
          }
      }
      

      After the proposed change, our test code can look like this:

      try (SomeManager mgr = new SomeManager(lots, of, params)) {
          // test code
      }
      

      New Javadoc:

      /**
       * Abstract base class used to register managers.
       * <p>
       * This class implements {@link AutoCloseable} mostly to allow unit tests to be written safely and succinctly. While
       * managers do need to allocate resources (usually on construction) and then free these resources, a manager is longer
       * lived than other auto-closeable objects like streams. None the less, making a manager AutoCloseable forces readers to
       * be aware of the the pattern: allocate resources on construction and call {@link #close()} at some point.
       * </p>
       */
      public abstract class AbstractManager implements AutoCloseable {
      

        Attachments

          Activity

            People

            • Assignee:
              garydgregory Gary Gregory
              Reporter:
              garydgregory Gary Gregory
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: