You are viewing a plain text version of this content. The canonical link for it is here.
Posted to log4j-dev@logging.apache.org by ca...@apache.org on 2006/03/16 06:33:44 UTC

svn commit: r386266 - in /logging/log4j/trunk: src/java/org/apache/log4j/AsyncAppender.java src/java/org/apache/log4j/Dispatcher.java src/java/org/apache/log4j/helpers/BoundedFIFO.java tests/src/java/org/apache/log4j/AsyncAppenderTestCase.java

Author: carnold
Date: Wed Mar 15 21:33:43 2006
New Revision: 386266

URL: http://svn.apache.org/viewcvs?rev=386266&view=rev
Log:
Bug 38982: Non-blocking option for AsyncAppender

Added:
    logging/log4j/trunk/src/java/org/apache/log4j/Dispatcher.java
Modified:
    logging/log4j/trunk/src/java/org/apache/log4j/AsyncAppender.java
    logging/log4j/trunk/src/java/org/apache/log4j/helpers/BoundedFIFO.java
    logging/log4j/trunk/tests/src/java/org/apache/log4j/AsyncAppenderTestCase.java

Modified: logging/log4j/trunk/src/java/org/apache/log4j/AsyncAppender.java
URL: http://svn.apache.org/viewcvs/logging/log4j/trunk/src/java/org/apache/log4j/AsyncAppender.java?rev=386266&r1=386265&r2=386266&view=diff
==============================================================================
--- logging/log4j/trunk/src/java/org/apache/log4j/AsyncAppender.java (original)
+++ logging/log4j/trunk/src/java/org/apache/log4j/AsyncAppender.java Wed Mar 15 21:33:43 2006
@@ -1,12 +1,12 @@
 /*
- * Copyright 1999,2004 The Apache Software Foundation.
- * 
+ * Copyright 1999,2006 The Apache Software Foundation.
+ *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
  * You may obtain a copy of the License at
- * 
+ *
  *      http://www.apache.org/licenses/LICENSE-2.0
- * 
+ *
  * Unless required by applicable law or agreed to in writing, software
  * distributed under the License is distributed on an "AS IS" BASIS,
  * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
@@ -14,41 +14,43 @@
  * limitations under the License.
  */
 
-
 // Contibutors:  Aaron Greenhouse <aa...@cs.cmu.edu>
 //               Thomas Tuft Muller <tt...@online.no>
 package org.apache.log4j;
 
 import org.apache.log4j.helpers.AppenderAttachableImpl;
-import org.apache.log4j.helpers.BoundedFIFO;
 import org.apache.log4j.spi.AppenderAttachable;
 import org.apache.log4j.spi.LoggingEvent;
 
+import java.text.MessageFormat;
+
+import java.util.ArrayList;
 import java.util.Enumeration;
+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
 
 
 /**
- * The AsyncAppender lets users log events asynchronously. It uses a bounded
- * buffer to store logging events.
- * 
- * <p>
+ * The AsyncAppender lets users log events asynchronously.
+ * <p/>
+ * <p/>
  * The AsyncAppender will collect the events sent to it and then dispatch them
  * to all the appenders that are attached to it. You can attach multiple
  * appenders to an AsyncAppender.
  * </p>
- * 
- * <p>
- * The AsyncAppender uses a separate thread to serve the events in its bounded
- * buffer.
+ * <p/>
+ * <p/>
+ * The AsyncAppender uses a separate thread to serve the events in its buffer.
  * </p>
-
- *  * <p>
+ * <p/>
  * <b>Important note:</b> The <code>AsyncAppender</code> can only be script
  * configured using the {@link org.apache.log4j.joran.JoranConfigurator}.
  * </p>
  *
  * @author Ceki G&uuml;lc&uuml;
- *
+ * @author Curt Arnold
  * @since 0.9.1
  */
 public class AsyncAppender extends AppenderSkeleton
@@ -58,67 +60,161 @@
    */
   public static final int DEFAULT_BUFFER_SIZE = 128;
 
-  //static Category cat = Category.getInstance(AsyncAppender.class.getName());
-  private BoundedFIFO bf = new BoundedFIFO(DEFAULT_BUFFER_SIZE);
+  /**
+   * Event buffer, also used as monitor to protect itself and
+   * discardMap from simulatenous modifications.
+   */
+  private final List buffer = new ArrayList();
+
+  /**
+   * Map of DiscardSummary objects keyed by logger name.
+   */
+  private final Map discardMap = new HashMap();
+
+  /**
+   * Buffer size.
+   */
+  private int bufferSize = DEFAULT_BUFFER_SIZE;
+
+  /** Nested appenders. */
   AppenderAttachableImpl aai;
-  private Dispatcher dispatcher;
+
+  /**
+   * Nested appenders.
+   */
+  private final AppenderAttachableImpl appenders;
+
+  /**
+   * Dispatcher.
+   */
+  private final Thread dispatcher;
+
+  /**
+   * Should location info be included in dispatched messages.
+   */
   private boolean locationInfo = false;
-  private boolean interruptedWarningMessage = false;
 
+  /**
+   * Does appender block when buffer is full.
+   */
+  private boolean blocking = true;
+
+  /**
+   * Create new instance.
+   */
   public AsyncAppender() {
     super(true);
-    // Note: The dispatcher code assumes that the aai is set once and
-    // for all.
-    aai = new AppenderAttachableImpl();
-    dispatcher = new Dispatcher(bf, this);
+    appenders = new AppenderAttachableImpl();
+
+    //
+    //   only set for compatibility
+    aai = appenders;
+
+    dispatcher =
+      new Thread(new Dispatcher(this, buffer, discardMap, appenders));
+
+    // It is the user's responsibility to close appenders before
+    // exiting.
+    dispatcher.setDaemon(true);
+
+    // set the dispatcher priority to lowest possible value
+    //        dispatcher.setPriority(Thread.MIN_PRIORITY);
+    dispatcher.setName("Dispatcher-" + dispatcher.getName());
     dispatcher.start();
   }
-  
-  public void addAppender(Appender newAppender) {
-    synchronized (aai) {
-      aai.addAppender(newAppender);
+
+  /**
+   * Add appender.
+   *
+   * @param newAppender appender to add, may not be null.
+   */
+  public void addAppender(final Appender newAppender) {
+    synchronized (appenders) {
+      appenders.addAppender(newAppender);
     }
   }
 
-  public void append(LoggingEvent event) {
+  /**
+   * {@inheritDoc}
+   */
+  public void append(final LoggingEvent event) {
     //
     //   if dispatcher thread has died then
     //      append subsequent events synchronously
     //   See bug 23021
-    if (!dispatcher.isAlive()) {
-        synchronized(aai) {
-            aai.appendLoopOnAppenders(event);
-        }
-        return;
+    if ((dispatcher == null) || !dispatcher.isAlive() || (bufferSize <= 0)) {
+      synchronized (appenders) {
+        appenders.appendLoopOnAppenders(event);
+      }
+
+      return;
     }
+
     // extract all the thread dependent information now as later it will
     // be too late.
     event.prepareForDeferredProcessing();
+
     if (locationInfo) {
       event.getLocationInformation();
     }
 
-    synchronized (bf) {
-      while (bf.isFull()) {
-        try {
-          //LogLog.debug("Waiting for free space in buffer, "+bf.length());
-          bf.wait();
-        } catch (InterruptedException e) {
-          if (!interruptedWarningMessage) {
-            interruptedWarningMessage = true;
-            getLogger().warn("AsyncAppender interrupted.", e);
-          } else {
-            getLogger().warn("AsyncAppender interrupted again.");
+    synchronized (buffer) {
+      while (true) {
+        int previousSize = buffer.size();
+
+        if (previousSize < bufferSize) {
+          buffer.add(event);
+
+          //
+          //   if buffer had been empty
+          //       signal all threads waiting on buffer
+          //       to check their conditions.
+          //
+          if (previousSize == 0) {
+            buffer.notifyAll();
+          }
+
+          break;
+        }
+
+        //
+        //   Following code is only reachable if buffer is full
+        //
+        //
+        //   if blocking and thread is not already interrupted
+        //      wait for a buffer notification
+        boolean discard = true;
+
+        if (blocking && !Thread.interrupted()) {
+          try {
+            buffer.wait();
+            discard = false;
+          } catch (InterruptedException e) {
+            //
+            //  reset interrupt status so
+            //    calling code can see interrupt on
+            //    their next wait or sleep.
+            Thread.currentThread().interrupt();
           }
         }
-      }
 
-      //cat.debug("About to put new event in buffer.");
-      bf.put(event);
+        //
+        //   if blocking is false or thread has been interrupted
+        //   add event to discard map.
+        //
+        if (discard) {
+          String loggerName = event.getLoggerName();
+          DiscardSummary summary = (DiscardSummary) discardMap.get(loggerName);
+
+          if (summary == null) {
+            summary = new DiscardSummary(event);
+            discardMap.put(loggerName, summary);
+          } else {
+            summary.add(event);
+          }
 
-      if (bf.wasEmpty()) {
-        //cat.debug("Notifying dispatcher to process events.");
-        bf.notify();
+          break;
+        }
       }
     }
   }
@@ -128,81 +224,118 @@
    * thread which will process all pending events before exiting.
    */
   public void close() {
-    synchronized (this) {
-      // avoid multiple close, otherwise one gets NullPointerException
-      if (closed) {
-        return;
-      }
-
+    /**
+     * Set closed flag and notify all threads to check their conditions.
+     * Should result in dispatcher terminating.
+     */
+    synchronized (buffer) {
       closed = true;
+      buffer.notifyAll();
     }
 
-    // The following cannot be synchronized on "this" because the
-    // dispatcher synchronizes with "this" in its while loop. If we
-    // did synchronize we would systematically get deadlocks when
-    // close was called.
-    dispatcher.close();
-
     try {
       dispatcher.join();
     } catch (InterruptedException e) {
+      Thread.currentThread().interrupt();
       getLogger().error(
         "Got an InterruptedException while waiting for the "
         + "dispatcher to finish.", e);
     }
 
-    dispatcher = null;
-    bf = null;
+    //
+    //    close all attached appenders.
+    //
+    synchronized (appenders) {
+      Enumeration iter = appenders.getAllAppenders();
+
+      if (iter != null) {
+        while (iter.hasMoreElements()) {
+          Object next = iter.nextElement();
+
+          if (next instanceof Appender) {
+            ((Appender) next).close();
+          }
+        }
+      }
+    }
   }
 
+  /**
+   * Get iterator over attached appenders.
+   * @return iterator or null if no attached appenders.
+   */
   public Enumeration getAllAppenders() {
-    synchronized (aai) {
-      return aai.getAllAppenders();
+    synchronized (appenders) {
+      return appenders.getAllAppenders();
     }
   }
 
-  public Appender getAppender(String name) {
-    synchronized (aai) {
-      return aai.getAppender(name);
+  /**
+   * Get appender by name.
+   *
+   * @param name name, may not be null.
+   * @return matching appender or null.
+   */
+  public Appender getAppender(final String name) {
+    synchronized (appenders) {
+      return appenders.getAppender(name);
     }
   }
 
   /**
-   * Returns the current value of the <b>LocationInfo</b> option.
+   * Gets whether the location of the logging request call
+   * should be captured.
+   *
+   * @return the current value of the <b>LocationInfo</b> option.
    */
   public boolean getLocationInfo() {
     return locationInfo;
   }
 
   /**
-   * Is the appender passed as parameter attached to this category?
+   * Determines if specified appender is attached.
+   * @param appender appender.
+   * @return true if attached.
    */
-  public boolean isAttached(Appender appender) {
-    return aai.isAttached(appender);
+  public boolean isAttached(final Appender appender) {
+    synchronized (appenders) {
+      return appenders.isAttached(appender);
+    }
   }
 
   /**
-   * @deprecated Will be removed with no replacement.
+   * {@inheritDoc}
    */
   public boolean requiresLayout() {
     return false;
   }
 
+  /**
+   * Removes and closes all attached appenders.
+   */
   public void removeAllAppenders() {
-    synchronized (aai) {
-      aai.removeAllAppenders();
+    synchronized (appenders) {
+      appenders.removeAllAppenders();
     }
   }
 
-  public void removeAppender(Appender appender) {
-    synchronized (aai) {
-      aai.removeAppender(appender);
+  /**
+   * Removes an appender.
+   * @param appender appender to remove.
+   */
+  public void removeAppender(final Appender appender) {
+    synchronized (appenders) {
+      appenders.removeAppender(appender);
     }
   }
 
-  public void removeAppender(String name) {
-    synchronized (aai) {
-      aai.removeAppender(name);
+  /**
+   * Remove appender by name.
+   * @param name name.
+   */
+  public void removeAppender(final String name) {
+    synchronized (appenders) {
+      appenders.removeAppender(name);
     }
   }
 
@@ -212,131 +345,241 @@
    * information related to the event. As a result, the event that will be
    * ultimately logged will likely to contain the wrong location information
    * (if present in the log format).
-   * 
-   * <p>
+   * <p/>
+   * <p/>
    * Location information extraction is comparatively very slow and should be
    * avoided unless performance is not a concern.
    * </p>
+   * @param flag true if location information should be extracted.
    */
-  public void setLocationInfo(boolean flag) {
+  public void setLocationInfo(final boolean flag) {
     locationInfo = flag;
   }
 
   /**
-   * The <b>BufferSize</b> option takes a non-negative integer value. This
-   * integer value determines the maximum size of the bounded buffer.
-   * Increasing the size of the buffer is always safe. However, if an
-   * existing buffer holds unwritten elements, then <em>decreasing the buffer
-   * size will result in event loss.</em> Nevertheless, while script
-   * configuring the AsyncAppender, it is safe to set a buffer size smaller
-   * than the {@link #DEFAULT_BUFFER_SIZE default buffer size} because
-   * configurators guarantee that an appender cannot be used before being
-   * completely configured.
+   * Sets the number of messages allowed in the event buffer
+   * before the calling thread is blocked (if blocking is true)
+   * or until messages are summarized and discarded.  Changing
+   * the size will not affect messages already in the buffer.
+   *
+   * @param size buffer size, must be positive.
    */
-  public void setBufferSize(int size) {
-    bf.resize(size);
+  public void setBufferSize(final int size) {
+    //
+    //   log4j 1.2 would throw exception if size was negative
+    //      and deadlock if size was zero.
+    //
+    if (size < 0) {
+      throw new java.lang.NegativeArraySizeException("size");
+    }
+
+    synchronized (buffer) {
+      //
+      //   don't let size be zero.
+      //
+      bufferSize = (size < 1) ? 1 : size;
+      buffer.notifyAll();
+    }
   }
 
   /**
-   * Returns the current value of the <b>BufferSize</b> option.
+   * Gets the current buffer size.
+   * @return the current value of the <b>BufferSize</b> option.
    */
   public int getBufferSize() {
-      // Bugzilla 23912
-      synchronized(bf) {  
-          return bf.getMaxSize();
-      }
+    return bufferSize;
   }
-}
 
+  /**
+   * Sets whether appender should wait if there is no
+   * space available in the event buffer or immediately return.
+   *
+   * @param value true if appender should wait until available space in buffer.
+   */
+  public void setBlocking(final boolean value) {
+    synchronized (buffer) {
+      blocking = value;
+      buffer.notifyAll();
+    }
+  }
 
-// ------------------------------------------------------------------------------
-// ------------------------------------------------------------------------------
-// ----------------------------------------------------------------------------
-class Dispatcher extends Thread {
-  private BoundedFIFO bf;
-  private AppenderAttachableImpl aai;
-  private boolean interrupted = false;
-  AsyncAppender container;
-
-  Dispatcher(BoundedFIFO bf, AsyncAppender container) {
-    this.bf = bf;
-    this.container = container;
-    this.aai = container.aai;
+  /**
+   * Gets whether appender should block calling thread when buffer is full.
+   * If false, messages will be counted by logger and a summary
+   * message appended after the contents of the buffer have been appended.
+   *
+   * @return true if calling thread will be blocked when buffer is full.
+   */
+  public boolean getBlocking() {
+    return blocking;
+  }
 
-    // It is the user's responsibility to close appenders before
-    // exiting. 
-    this.setDaemon(true);
+  /**
+   * Summary of discarded logging events for a logger.
+   */
+  private static final class DiscardSummary {
+    /**
+     * First event of the highest severity.
+     */
+    private LoggingEvent maxEvent;
+
+    /**
+     * Total count of messages discarded.
+     */
+    private int count;
+
+    /**
+     * Create new instance.
+     *
+     * @param event event, may not be null.
+     */
+    public DiscardSummary(final LoggingEvent event) {
+      maxEvent = event;
+      count = 1;
+    }
+
+    /**
+     * Add discarded event to summary.
+     *
+     * @param event event, may not be null.
+     */
+    public void add(final LoggingEvent event) {
+      if (event.getLevel().toInt() > maxEvent.getLevel().toInt()) {
+        maxEvent = event;
+      }
 
-    // set the dispatcher priority to lowest possible value
-    this.setPriority(Thread.MIN_PRIORITY);
-    this.setName("Dispatcher-" + getName());
+      count++;
+    }
 
-    // set the dispatcher priority to MIN_PRIORITY plus or minus 2
-    // depending on the direction of MIN to MAX_PRIORITY.
-    //+ (Thread.MAX_PRIORITY > Thread.MIN_PRIORITY ? 1 : -1)*2);
-  }
-
-  void close() {
-    synchronized (bf) {
-      interrupted = true;
-
-      // We have a waiting dispacther if and only if bf.length is
-      // zero.  In that case, we need to give it a death kiss.
-      if (bf.length() == 0) {
-        bf.notify();
-      }
+    /**
+     * Create event with summary information.
+     *
+     * @return new event.
+     */
+    public LoggingEvent createEvent() {
+      String msg =
+        MessageFormat.format(
+          "Discarded {0} messages due to full event buffer including: {1}",
+          new Object[] { new Integer(count), maxEvent.getMessage() });
+
+      return new LoggingEvent(
+        null, maxEvent.getLogger(), maxEvent.getLevel(), msg, null);
     }
   }
 
   /**
-   * The dispatching strategy is to wait until there are events in the buffer
-   * to process. After having processed an event, we release the monitor
-   * (variable bf) so that new events can be placed in the buffer, instead of
-   * keeping the monitor and processing the remaining events in the buffer.
-   * 
-   * <p>
-   * Other approaches might yield better results.
-   * </p>
+   * Event dispatcher.
    */
-  public void run() {
-    //Category cat = Category.getInstance(Dispatcher.class.getName());
-    LoggingEvent event;
-
-    while (true) {
-      synchronized (bf) {
-        if (bf.length() == 0) {
-          // Exit loop if interrupted but only if the the buffer is empty.
-          if (interrupted) {
-            //cat.info("Exiting.");
-            break;
+  private static class Dispatcher implements Runnable {
+    /**
+     * Parent AsyncAppender.
+     */
+    private final AsyncAppender parent;
+
+    /**
+     * Event buffer.
+     */
+    private final List buffer;
+
+    /**
+     * Map of DiscardSummary keyed by logger name.
+     */
+    private final Map discardMap;
+
+    /**
+     * Wrapped appenders.
+     */
+    private final AppenderAttachableImpl appenders;
+
+    /**
+     * Create new instance of dispatcher.
+     *
+     * @param parent     parent AsyncAppender, may not be null.
+     * @param buffer     event buffer, may not be null.
+     * @param discardMap discard map, may not be null.
+     * @param appenders  appenders, may not be null.
+     */
+    public Dispatcher(
+      final AsyncAppender parent, final List buffer, final Map discardMap,
+      final AppenderAttachableImpl appenders) {
+
+      this.parent = parent;
+      this.buffer = buffer;
+      this.appenders = appenders;
+      this.discardMap = discardMap;
+    }
+
+    /**
+     * {@inheritDoc}
+     */
+    public void run() {
+      boolean isActive = true;
+
+      //
+      //   if interrupted (unlikely), end thread
+      //
+      try {
+        //
+        //   loop until the AsyncAppender is closed.
+        //
+        while (isActive) {
+          LoggingEvent[] events = null;
+
+          //
+          //   extract pending events while synchronized
+          //       on buffer
+          //
+          synchronized (buffer) {
+            int bufferSize = buffer.size();
+            isActive = !parent.isClosed();
+
+            while ((bufferSize == 0) && isActive) {
+              buffer.wait();
+              bufferSize = buffer.size();
+              isActive = !parent.isClosed();
+            }
+
+            if (bufferSize > 0) {
+              events = new LoggingEvent[bufferSize + discardMap.size()];
+              buffer.toArray(events);
+
+              //
+              //   add events due to buffer overflow
+              //
+              int index = bufferSize;
+
+              for (
+                Iterator iter = discardMap.values().iterator();
+                  iter.hasNext();) {
+                events[index++] = ((DiscardSummary) iter.next()).createEvent();
+              }
+
+              //
+              //    clear buffer and discard map
+              //
+              buffer.clear();
+              discardMap.clear();
+
+              //
+              //    allow blocked appends to continue
+              buffer.notifyAll();
+            }
           }
 
-          try {
-            //LogLog.debug("Waiting for new event to dispatch.");
-            bf.wait();
-          } catch (InterruptedException e) {
-            break;
+          //
+          //   process events after lock on buffer is released.
+          //
+          if (events != null) {
+            for (int i = 0; i < events.length; i++) {
+              synchronized (appenders) {
+                appenders.appendLoopOnAppenders(events[i]);
+              }
+            }
           }
         }
-
-        event = bf.get();
-
-        if (bf.wasFull()) {
-          //LogLog.debug("Notifying AsyncAppender about freed space.");
-          bf.notify();
-        }
-      }
-       // synchronized
-
-      synchronized (container.aai) {
-        if ((aai != null) && (event != null)) {
-          aai.appendLoopOnAppenders(event);
-        }
+      } catch (InterruptedException ex) {
+        Thread.currentThread().interrupt();
       }
     }
-     // while
-
-    // close and remove all appenders
-    aai.removeAllAppenders();
   }
 }

Added: logging/log4j/trunk/src/java/org/apache/log4j/Dispatcher.java
URL: http://svn.apache.org/viewcvs/logging/log4j/trunk/src/java/org/apache/log4j/Dispatcher.java?rev=386266&view=auto
==============================================================================
--- logging/log4j/trunk/src/java/org/apache/log4j/Dispatcher.java (added)
+++ logging/log4j/trunk/src/java/org/apache/log4j/Dispatcher.java Wed Mar 15 21:33:43 2006
@@ -0,0 +1,124 @@
+/*
+ * Copyright 1999,2005 The Apache Software Foundation.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.log4j;
+
+import org.apache.log4j.helpers.AppenderAttachableImpl;
+import org.apache.log4j.spi.LoggingEvent;
+
+
+/**
+ * Obsolete AsyncAppender dispatcher provided for compatibility only.
+ *
+ * @deprecated Since 1.3.
+ */
+class Dispatcher extends Thread {
+    /**
+     * @deprecated
+     */
+  private org.apache.log4j.helpers.BoundedFIFO bf;
+  private AppenderAttachableImpl aai;
+  private boolean interrupted = false;
+  AsyncAppender container;
+
+    /**
+     *
+     * @param bf
+     * @param container
+     * @deprecated
+     */
+  Dispatcher(org.apache.log4j.helpers.BoundedFIFO bf, AsyncAppender container) {
+    this.bf = bf;
+    this.container = container;
+    this.aai = container.aai;
+
+    // It is the user's responsibility to close appenders before
+    // exiting.
+    this.setDaemon(true);
+
+    // set the dispatcher priority to lowest possible value
+    this.setPriority(Thread.MIN_PRIORITY);
+    this.setName("Dispatcher-" + getName());
+
+    // set the dispatcher priority to MIN_PRIORITY plus or minus 2
+    // depending on the direction of MIN to MAX_PRIORITY.
+    //+ (Thread.MAX_PRIORITY > Thread.MIN_PRIORITY ? 1 : -1)*2);
+  }
+
+  void close() {
+    synchronized (bf) {
+      interrupted = true;
+
+      // We have a waiting dispacther if and only if bf.length is
+      // zero.  In that case, we need to give it a death kiss.
+      if (bf.length() == 0) {
+        bf.notify();
+      }
+    }
+  }
+
+  /**
+   * The dispatching strategy is to wait until there are events in the buffer
+   * to process. After having processed an event, we release the monitor
+   * (variable bf) so that new events can be placed in the buffer, instead of
+   * keeping the monitor and processing the remaining events in the buffer.
+   *
+   * <p>
+   * Other approaches might yield better results.
+   * </p>
+   */
+  public void run() {
+    //Category cat = Category.getInstance(Dispatcher.class.getName());
+    LoggingEvent event;
+
+    while (true) {
+      synchronized (bf) {
+        if (bf.length() == 0) {
+          // Exit loop if interrupted but only if the the buffer is empty.
+          if (interrupted) {
+            //cat.info("Exiting.");
+            break;
+          }
+
+          try {
+            //LogLog.debug("Waiting for new event to dispatch.");
+            bf.wait();
+          } catch (InterruptedException e) {
+            break;
+          }
+        }
+
+        event = bf.get();
+
+        if (bf.wasFull()) {
+          //LogLog.debug("Notifying AsyncAppender about freed space.");
+          bf.notify();
+        }
+      }
+
+      // synchronized
+      synchronized (container.aai) {
+        if ((aai != null) && (event != null)) {
+          aai.appendLoopOnAppenders(event);
+        }
+      }
+    }
+
+    // while
+    // close and remove all appenders
+    aai.removeAllAppenders();
+  }
+}

Modified: logging/log4j/trunk/src/java/org/apache/log4j/helpers/BoundedFIFO.java
URL: http://svn.apache.org/viewcvs/logging/log4j/trunk/src/java/org/apache/log4j/helpers/BoundedFIFO.java?rev=386266&r1=386265&r2=386266&view=diff
==============================================================================
--- logging/log4j/trunk/src/java/org/apache/log4j/helpers/BoundedFIFO.java (original)
+++ logging/log4j/trunk/src/java/org/apache/log4j/helpers/BoundedFIFO.java Wed Mar 15 21:33:43 2006
@@ -23,10 +23,14 @@
 
 /**
    <code>BoundedFIFO</code> serves as the bounded first-in-first-out
-   buffer heavily used by the {@link org.apache.log4j.AsyncAppender}.
+   buffer previously used by the {@link org.apache.log4j.AsyncAppender}.
    
    @author Ceki G&uuml;lc&uuml; 
-   @since version 0.9.1 */
+   @since version 0.9.1
+
+   @deprecated Since 1.3.
+
+ */
 public class BoundedFIFO {
   
   LoggingEvent[] buf;

Modified: logging/log4j/trunk/tests/src/java/org/apache/log4j/AsyncAppenderTestCase.java
URL: http://svn.apache.org/viewcvs/logging/log4j/trunk/tests/src/java/org/apache/log4j/AsyncAppenderTestCase.java?rev=386266&r1=386265&r2=386266&view=diff
==============================================================================
--- logging/log4j/trunk/tests/src/java/org/apache/log4j/AsyncAppenderTestCase.java (original)
+++ logging/log4j/trunk/tests/src/java/org/apache/log4j/AsyncAppenderTestCase.java Wed Mar 15 21:33:43 2006
@@ -27,6 +27,8 @@
 /**
  *  Tests for AsyncAppender.
  *
+ * @author Curt Arnold
+ *
  */
 public final class AsyncAppenderTestCase extends TestCase {
   /**
@@ -100,7 +102,7 @@
     root.debug("m2");
 
     Vector v = vectorAppender.getVector();
-    assertEquals(v.size(), 1);
+    assertEquals(1, v.size());
     assertTrue(vectorAppender.isClosed());
   }
 
@@ -120,7 +122,7 @@
     //  NullPointerException should kill dispatching thread
     //     before sleep returns.
     root.info("Message");
-    Thread.sleep(200);
+    Thread.sleep(100);
 
     try {
       //
@@ -146,7 +148,7 @@
     //
     final int threadCount = 10;
     Thread[] threads = new Thread[threadCount];
-    final int repetitions = 100;
+    final int repetitions = 20;
     Greeter greeter = new Greeter(root, repetitions);
 
     for (int i = 0; i < threads.length; i++) {
@@ -188,19 +190,14 @@
     Thread greeter = new Thread(new Greeter(root, 100));
 
     synchronized (blockableAppender.getMonitor()) {
+      //  Start greeter
       greeter.start();
+
+      //   Give it enough time to fill buffer
       Thread.sleep(100);
 
       //
-      //   Undesirable behavior: Interrupts are swallowed by
-      //   AsycnAppender which could interfere with expected
-      //   response to interrupts if the client code called wait or
-      //   sleep.
-      //
-      greeter.interrupt();
-      Thread.sleep(10);
-      greeter.interrupt();
-      Thread.sleep(10);
+      //   Interrupt should stop greeter after next logging event
       greeter.interrupt();
     }
 
@@ -208,7 +205,12 @@
     asyncAppender.close();
 
     Vector events = blockableAppender.getVector();
-    assertEquals(100, events.size());
+
+    //
+    //   1 popped off of buffer by dispatcher before being blocked
+    //   5 in buffer before it filled up
+    //   1 before Thread.sleep in greeter
+    assertEquals(7, events.size());
   }
 
   /**
@@ -257,13 +259,16 @@
     assertNotNull(dispatcher);
     dispatcher.interrupt();
     Thread.sleep(50);
+    root.info("Hello, World");
 
     //
-    //   Undesirable action: interrupting the dispatch thread
-    //        removes all appenders.
-    //
+    //  interrupting the dispatch thread should
+    //     degrade to synchronous dispatching of logging requests
     Enumeration iter = asyncAppender.getAllAppenders();
-    assertTrue((iter == null) || !iter.hasMoreElements());
+    assertTrue(iter.hasMoreElements());
+    assertSame(blockableAppender, iter.nextElement());
+    assertFalse(iter.hasMoreElements());
+    assertEquals(2, blockableAppender.getVector().size());
   }
 
   /**
@@ -281,16 +286,16 @@
   public void testSetBufferSizeZero() {
     VectorAppender vectorAppender = createDelayedAppender();
     asyncAppender = createAsyncAppender(vectorAppender, 0);
-    assertEquals(0, asyncAppender.getBufferSize());
+    assertEquals(1, asyncAppender.getBufferSize());
 
     //
     //   any logging request will deadlock.
-    //root.debug("m1");
-    //root.debug("m2");
+    root.debug("m1");
+    root.debug("m2");
     asyncAppender.close();
 
     Vector v = vectorAppender.getVector();
-    assertEquals(v.size(), 0);
+    assertEquals(2, v.size());
   }
 
   /**
@@ -408,6 +413,40 @@
     assertFalse(asyncAppender.getAllAppenders().hasMoreElements());
   }
 
+    /**
+     * Tests discarding of messages when buffer is full.
+     */
+    public void testDiscard() {
+        BlockableVectorAppender blockableAppender = new BlockableVectorAppender();
+        asyncAppender = createAsyncAppender(blockableAppender, 5);
+        assertTrue(asyncAppender.getBlocking());
+        asyncAppender.setBlocking(false);
+        assertFalse(asyncAppender.getBlocking());
+        Greeter greeter = new Greeter(root, 100);
+        synchronized(blockableAppender.getMonitor()) {
+            greeter.run();
+            root.error("That's all folks.");
+        }
+        asyncAppender.close();
+        Vector events = blockableAppender.getVector();
+        //
+        //  1 event pulled from buffer by dispatcher before blocking
+        //  5 events in buffer
+        //  1 summary event
+        //
+        assertEquals(7, events.size());
+        //
+        //  last message should start with "Discarded"
+        LoggingEvent event = (LoggingEvent) events.get(events.size() - 1);
+        assertEquals("Discarded", event.getMessage().toString().substring(0, 9));
+        assertSame(Level.ERROR, event.getLevel());
+        for (int i = 0; i < events.size() - 1; i++) {
+            event = (LoggingEvent) events.get(i);
+            assertEquals("Hello, World", event.getMessage().toString());
+        }
+    }
+
+
   /**
    * Appender that throws a NullPointerException on calls to append.
    * Used to test behavior of AsyncAppender when dispatching to
@@ -474,10 +513,13 @@
      * {@inheritDoc}
      */
     public void run() {
-      synchronized (this) {
-        for (int i = 0; (i < repetitions) && !Thread.interrupted(); i++) {
+      try {
+        for (int i = 0; i < repetitions; i++) {
           logger.info("Hello, World");
+          Thread.sleep(1);
         }
+      } catch (InterruptedException ex) {
+        Thread.currentThread().interrupt();
       }
     }
   }



---------------------------------------------------------------------
To unsubscribe, e-mail: log4j-dev-unsubscribe@logging.apache.org
For additional commands, e-mail: log4j-dev-help@logging.apache.org