[logback-dev] svn commit: r1868 - logback/trunk/logback-core/src/main/java/ch/qos/logback/core/spi

[Joern Huxhorn]

noreply.ceki at qos.ch wrote:
> Author: ceki
> Date: Thu Oct 23 22:02:53 2008
> New Revision: 1868
>
> Modified:
>    logback/trunk/logback-core/src/main/java/ch/qos/logback/core/spi/AppenderAttachableImpl.java
>
> Log:
> LBCORE-63
>
> Using ReadWriteLock  with the hope of improving performance.
>
>   

>    /**
>     * Attach an appender. If the appender is already in the list in won't be
> @@ -31,7 +38,12 @@
>      if (newAppender == null) {
>        throw new IllegalArgumentException("Null argument disallowed");
>      }
> -    appenderList.addIfAbsent(newAppender);
> +    w.lock();
> +    if (!appenderList.contains(newAppender)) {
> +      appenderList.add(newAppender);
> +    }
> +    w.unlock();
> +
>    }
>   
The unlock of a lock should, I would even say "must", always be done in
a finally block. Otherwise really bad things can happen if an exception
is thrown.

So the above code should look like this:

    w.lock();
    try {
      if (!appenderList.contains(newAppender)) {
        appenderList.add(newAppender);
      }
    }
    finally {
      w.unlock();
    }


The rest of the code should be changed accordingly, e.g.

  public boolean detachAppender(String name) {
    if (name == null) {
      return false;
    }
    w.lock();
    boolean removed=false;
    try {
      for (a : appenderList) {
        if (name.equals((a).getName())) {
          removed=appenderList.remove(a);
        }
      }
    }
    finally {
      w.unlock();
    }
    return removed;
  }