Tests and fixes for the DemuxOutputStream

1. New tests for the DemuxOutputStream and parallel task
2. Fix for the DemuxOutputStream thread problem identified by Jon Skeet
3. Found additional bug where UnknownElement does not hand output and error
strings onto the real task

A little bit of checkstyling.

Hmmm, really should be in bed.


git-svn-id: https://svn.apache.org/repos/asf/ant/core/trunk@271501 13f79535-47bb-0310-9956-ffa450edef68

src/etc/testcases/taskdefs/parallel.xml

@@ -0,0 +1,38 @@
+<?xml version="1.0"?>
+
+<project name="parallel-test" basedir="." default="help">
+  <target name="testBasic">
+    <parallel>
+      <sequential>
+        <sleep seconds="1"/>
+        <echo message="${test.delayed}"/>
+      </sequential>
+      <echo message="${test.direct}"/>
+    </parallel>
+  </target>
+
+  <target name="testFail">
+    <parallel>
+      <sequential>
+        <sleep seconds="1"/>
+        <echo message="${test.delayed}"/>
+      </sequential>
+      <fail message="${test.failure}"/>
+    </parallel>
+  </target>
+
+  <target name="testDemux">
+    <parallel>
+      <demuxtest/>
+      <demuxtest/>
+      <demuxtest/>
+      <demuxtest/>
+      <demuxtest/>
+    </parallel>
+  </target>
+  
+  <target name="help">
+    <echo>Test build file for the &lt;parallel&gt; task.</echo> 
+    <echo>Use the various targets to run the tests.</echo>
+  </target>
+</project>

src/main/org/apache/tools/ant/DemuxOutputStream.java

@@ -64,25 +64,39 @@ import java.util.Hashtable;
  * project object which will forward the content to the appropriate
  * task.
  *
+ * @since 1.4
  * @author Conor MacNeill
  */
 public class DemuxOutputStream extends OutputStream {
 
+    /**
+     * A data class to store information about a buffer. Such informatio
+     * is stored on a per-thread basis.
+     */
+    private static class BufferInfo {
+        /**
+         * The per-thread output stream
+         */
+        private ByteArrayOutputStream buffer;
+        
+        /** 
+         * Whether the next line-terminator should be skipped in terms
+         * of processing the buffer or not. Used to avoid \r\n invoking
+         * processBuffer twice.
+         */
+         private boolean skip = false;
+    }
+    
     /** Maximum buffer size */
     private final static int MAX_SIZE = 1024;
-    /** Mapping from thread to buffer (Thread to ByteOutputStream) */
+    /** Mapping from thread to buffer (Thread to BufferInfo) */
     private Hashtable buffers = new Hashtable();
-//    private ByteArrayOutputStream buffer = new ByteArrayOutputStream();
-    /** 
-     * Whether the next line-terminator should be skipped in terms
-     * of processing the buffer or not. Used to avoid \r\n invoking
-     * processBuffer twice.
-     */
-    private boolean skip = false;
+
     /**
      * The project to send output to
      */
     private Project project;
+
     /**
      * Whether or not this stream represents an error stream
      */
@@ -91,8 +105,11 @@ public class DemuxOutputStream extends OutputStream {
     /**
      * Creates a new instance of this class.
      *
-     * @param task the task for whom to log
-     * @param level loglevel used to log data written to this stream.
+     * @param project the project instance for which output is being 
+     * demultiplexed.
+     * @param isErrorStream true if this is the error string, otherwise 
+     * a normal output stream. This is passed to the project so it knows
+     * which stream it is receiving.
      */
     public DemuxOutputStream(Project project, boolean isErrorStream) {
         this.project = project;
@@ -104,20 +121,22 @@ public class DemuxOutputStream extends OutputStream {
      * 
      * @return a ByteArrayOutputStream for the current thread to write data to
      */
-    private ByteArrayOutputStream getBuffer() {
+    private BufferInfo getBufferInfo() {
         Thread current = Thread.currentThread();
-        ByteArrayOutputStream buffer = (ByteArrayOutputStream)buffers.get(current);
-        if (buffer == null) {
-            buffer = new ByteArrayOutputStream();
-            buffers.put(current, buffer);
+        BufferInfo bufferInfo = (BufferInfo)buffers.get(current);
+        if (bufferInfo == null) {
+            bufferInfo = new BufferInfo();
+            bufferInfo.buffer = new ByteArrayOutputStream();
+            bufferInfo.skip = false;
+            buffers.put(current, bufferInfo);
         }
-        return buffer;
+        return bufferInfo;
     }
 
     /**
      * Resets the buffer for the current thread.
      */
-    private void resetBuffer() {    
+    private void resetBufferInfo() {    
         Thread current = Thread.currentThread();
         buffers.remove(current);
     }
@@ -127,44 +146,43 @@ public class DemuxOutputStream extends OutputStream {
      * separator is detected or if the buffer has reached its maximum size.
      *
      * @param cc data to log (byte).
+     * @exception IOException if the data cannot be written to the stream
      */
     public void write(int cc) throws IOException {
         final byte c = (byte)cc;
+
+        BufferInfo bufferInfo = getBufferInfo();
         if ((c == '\n') || (c == '\r')) {
-            if (!skip) {
-                processBuffer();
+            if (!bufferInfo.skip) {
+                processBuffer(bufferInfo.buffer);
             }
         } else {
-            ByteArrayOutputStream buffer = getBuffer();
-            buffer.write(cc);
-            if (buffer.size() > MAX_SIZE) {
-                processBuffer();
+            bufferInfo.buffer.write(cc);
+            if (bufferInfo.buffer.size() > MAX_SIZE) {
+                processBuffer(bufferInfo.buffer);
             }
         }
-       // XXX: This isn't threadsafe. Consider two threads logging
-       // Hello\r\n
-       // and
-       // There\r\n
-       // at the same time, with the two '\r's both being sent before
-       // either '\n', and the '\n's coming in the opposite order (thread-wise)
-       // to the '\r's - one buffer will be processed twice, and the other won't
-       // be processed at all.
-       skip = (c == '\r');
+        bufferInfo.skip = (c == '\r');
     }
 
 
     /**
      * Converts the buffer to a string and sends it to 
      * {@link Project#demuxOutput(String,boolean) Project.demuxOutput}.
+     *
+     * @param buffer the ByteArrayOutputStream used to collect the output
+     * until a line separator is seen.
      */
-    protected void processBuffer() {
-        String output = getBuffer().toString();
+    protected void processBuffer(ByteArrayOutputStream buffer) {
+        String output = buffer.toString();
         project.demuxOutput(output, isErrorStream);
-        resetBuffer();
+        resetBufferInfo();
     }
 
     /**
      * Equivalent to calling {@link #flush flush} on the stream.
+     *
+     * @exception IOException if there is a problem closing the stream.
      */
     public void close() throws IOException {
         flush();
@@ -173,10 +191,13 @@ public class DemuxOutputStream extends OutputStream {
     /**
      * Writes all remaining data in the buffer associated
      * with the current thread to the project.
+     *
+     * @exception IOException if there is a problem flushing the stream.
      */
     public void flush() throws IOException {
-        if (getBuffer().size() > 0) {
-            processBuffer();
+        BufferInfo bufferInfo = getBufferInfo();
+        if (bufferInfo.buffer.size() > 0) {
+            processBuffer(bufferInfo.buffer);
         }
     }
 }

src/main/org/apache/tools/ant/UnknownElement.java

@@ -80,12 +80,17 @@ public class UnknownElement extends Task {
      */
     private Vector children = new Vector();
 
+    /**
+     * Create an UnknownElement for the given element name.
+     *
+     * @param elementName the name of the unknown element.
+     */
     public UnknownElement (String elementName) {
         this.elementName = elementName;
     }
 
     /**
-     * return the corresponding XML element name.
+     * @return the corresponding XML element name.
      */
     public String getTag() {
         return elementName;
@@ -94,6 +99,8 @@ public class UnknownElement extends Task {
     /**
      * creates the real object instance, creates child elements, configures
      * the attributes of the real object.
+     *
+     * @exception BuildException if the configuration fails
      */
     public void maybeConfigure() throws BuildException {
         realThing = makeObject(this, wrapper);
@@ -113,6 +120,32 @@ public class UnknownElement extends Task {
         }
     }
 
+    /**
+     * Handle output sent to System.out by this task or its real task.
+     *
+     * @param line the output string
+     */
+    protected void handleOutput(String line) {
+        if (realThing instanceof Task) {
+            ((Task)realThing).handleOutput(line);
+        } else {
+            super.handleOutput(line);
+        }
+    }
+    
+    /**
+     * Handle error output sent to System.err by this task or its real task.
+     *
+     * @param line the error string
+     */
+    protected void handleErrorOutput(String line) {
+        if (realThing instanceof Task) {
+            ((Task)realThing).handleErrorOutput(line);
+        } else {
+            super.handleErrorOutput(line);
+        }
+    }
+    
     /**
      * Called when the real task has been configured for the first time.
      */
@@ -131,6 +164,8 @@ public class UnknownElement extends Task {
 
     /**
      * Adds a child element to this element.
+     *
+     * @param child the child element
      */
     public void addChild(UnknownElement child) {
         children.addElement(child);
@@ -139,6 +174,9 @@ public class UnknownElement extends Task {
     /**
      * Creates child elements, creates children of the children, sets
      * attributes of the child elements.
+     *
+     * @param parent the configured object for the parent
+     * @exception BuildException if the children cannot be configured.
      */
     protected void handleChildren(Object parent,
                                   RuntimeConfigurable parentWrapper)
@@ -151,7 +189,7 @@ public class UnknownElement extends Task {
         Class parentClass = parent.getClass();
         IntrospectionHelper ih = IntrospectionHelper.getHelper(parentClass);
 
-        for (int i=0; i<children.size(); i++) {
+        for (int i = 0;  i < children.size(); i++) {
             RuntimeConfigurable childWrapper = parentWrapper.getChild(i);
             UnknownElement child = (UnknownElement) children.elementAt(i);
             Object realChild = null;
@@ -177,7 +215,8 @@ public class UnknownElement extends Task {
     }
 
     /**
-     * Creates a named task or data type - if it is a task, configure it up to the init() stage.
+     * Creates a named task or data type - if it is a task, 
+     * configure it up to the init() stage.
      */
     protected Object makeObject(UnknownElement ue, RuntimeConfigurable w) {
         Object o = makeTask(ue, w, true);
@@ -213,35 +252,35 @@ public class UnknownElement extends Task {
                                                   String elementName) {
         String lSep = System.getProperty("line.separator");
         String msg = "Could not create " + what + " of type: " + elementName
-            + "." + lSep+ lSep
+            + "." + lSep + lSep
             + "Ant could not find the task or a class this "
-            + "task relies upon." + lSep +lSep
+            + "task relies upon." + lSep + lSep
             + "This is common and has a number of causes; the usual " + lSep
             + "solutions are to read the manual pages then download and" + lSep
-            + "install needed JAR files, or fix the build file: "+ lSep
+            + "install needed JAR files, or fix the build file: " + lSep
             + " - You have misspelt '" + elementName + "'." + lSep
-            + "   Fix: check your spelling." +lSep
-            + " - The task needs an external JAR file to execute" +lSep
-            + "   and this is not found at the right place in the classpath." +lSep
-            + "   Fix: check the documentation for dependencies." +lSep
-            + "   Fix: declare the task." +lSep
-            + " - The task is an Ant optional task and optional.jar is absent"+lSep
-            + "   Fix: look for optional.jar in ANT_HOME/lib, download if needed" +lSep
-            + " - The task was not built into optional.jar as dependent"  +lSep
+            + "   Fix: check your spelling." + lSep
+            + " - The task needs an external JAR file to execute" + lSep
+            + "   and this is not found at the right place in the classpath." + lSep
+            + "   Fix: check the documentation for dependencies." + lSep
+            + "   Fix: declare the task." + lSep
+            + " - The task is an Ant optional task and optional.jar is absent" + lSep
+            + "   Fix: look for optional.jar in ANT_HOME/lib, download if needed" + lSep
+            + " - The task was not built into optional.jar as dependent"  + lSep
             + "   libraries were not found at build time." + lSep
-            + "   Fix: look in the JAR to verify, then rebuild with the needed" +lSep
-            + "   libraries, or download a release version from apache.org" +lSep
-            + " - The build file was written for a later version of Ant" +lSep
-            + "   Fix: upgrade to at least the latest release version of Ant" +lSep
-            + " - The task is not an Ant core or optional task " +lSep
-            + "   and needs to be declared using <taskdef>." +lSep
+            + "   Fix: look in the JAR to verify, then rebuild with the needed" + lSep
+            + "   libraries, or download a release version from apache.org" + lSep
+            + " - The build file was written for a later version of Ant" + lSep
+            + "   Fix: upgrade to at least the latest release version of Ant" + lSep
+            + " - The task is not an Ant core or optional task " + lSep
+            + "   and needs to be declared using <taskdef>." + lSep
+            + lSep 
+            + "Remember that for JAR files to be visible to An t tasks implemented" + lSep
+            + "in ANT_HOME/lib, the files must be in the same directory or on the" + lSep
+            + "classpath" + lSep
             + lSep
-            + "Remember that for JAR files to be visible to Ant tasks implemented" +lSep
-            + "in ANT_HOME/lib, the files must be in the same directory or on the" +lSep
-            + "classpath"+ lSep
-            + lSep
-            + "Please neither file bug reports on this problem, nor email the" +lSep
-            + "Ant mailing lists, until all of these causes have been explored," +lSep
+            + "Please neither file bug reports on this problem, nor email the" + lSep
+            + "Ant mailing lists, until all of these causes have been explored," + lSep
             + "as this is not an Ant bug.";
 
 
@@ -259,7 +298,9 @@ public class UnknownElement extends Task {
     }
 
     /**
-     * Return the task instance after it has been created (and if it is a task.
+     * Return the task instance after it has been created and if it is a task.
+     *
+     * @return a task instance or null if the real object is not a task
      */
     public Task getTask() {
         if (realThing != null && realThing instanceof Task) {

src/testcases/org/apache/tools/ant/taskdefs/DemuxOutputTask.java

@@ -0,0 +1,110 @@
+/*
+ * The Apache Software License, Version 1.1
+ *
+ * Copyright (c) 2001 The Apache Software Foundation.  All rights
+ * reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ *
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in
+ *    the documentation and/or other materials provided with the
+ *    distribution.
+ *
+ * 3. The end-user documentation included with the redistribution, if
+ *    any, must include the following acknowlegement:
+ *       "This product includes software developed by the
+ *        Apache Software Foundation (http://www.apache.org/)."
+ *    Alternately, this acknowlegement may appear in the software itself,
+ *    if and wherever such third-party acknowlegements normally appear.
+ *
+ * 4. The names "The Jakarta Project", "Ant", and "Apache Software
+ *    Foundation" must not be used to endorse or promote products derived
+ *    from this software without prior written permission. For written
+ *    permission, please contact apache@apache.org.
+ *
+ * 5. Products derived from this software may not be called "Apache"
+ *    nor may "Apache" appear in their names without prior written
+ *    permission of the Apache Group.
+ *
+ * THIS SOFTWARE IS PROVIDED ``AS IS'' AND ANY EXPRESSED OR IMPLIED
+ * WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+ * DISCLAIMED.  IN NO EVENT SHALL THE APACHE SOFTWARE FOUNDATION OR
+ * ITS CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
+ * USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
+ * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+ * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
+ * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ * ====================================================================
+ *
+ * This software consists of voluntary contributions made by many
+ * individuals on behalf of the Apache Software Foundation.  For more
+ * information on the Apache Software Foundation, please see
+ * <http://www.apache.org/>.
+ */
+package org.apache.tools.ant.taskdefs;
+
+import org.apache.tools.ant.*;
+import org.apache.tools.ant.BuildFileTest;
+import java.util.Random;
+/**
+ * A simple task that prints to System.out and System.err and then catches
+ * the output which it then check. If the output does not match, an
+ * exception is thrown
+ *
+ * @since 1.5
+ * @author Conor MacNeill
+ * @created 21 February 2002
+ */
+public class DemuxOutputTask extends Task {
+    private String randomOutValue;
+    private String randomErrValue;
+    private boolean outputReceived = false;
+    private boolean errorReceived = false;
+    
+    public void execute() {
+        Random generator = new Random();
+        randomOutValue = "Output Value is " + generator.nextInt();
+        randomErrValue = "Error Value is " + generator.nextInt();
+        
+        System.out.println(randomOutValue);
+        System.err.println(randomErrValue);
+        if (!outputReceived) {
+            throw new BuildException("Did not receive output");
+        } 
+        
+        if (!errorReceived) {
+            throw new BuildException("Did not receive error");
+        }
+    }
+
+    protected void handleOutput(String line) {
+        if (!line.equals(randomOutValue)) {
+            String message = "Received = [" + line + "], expected = [" 
+                + randomOutValue + "]";
+            throw new BuildException(message);
+        }
+        outputReceived = true;
+    }
+    
+    protected void handleErrorOutput(String line) {
+        if (!line.equals(randomErrValue)) {
+            String message = "Received = [" + line + "], expected = [" 
+                + randomErrValue + "]";
+            throw new BuildException(message);
+        }
+        errorReceived = true;
+    }
+    
+    
+}
+

src/testcases/org/apache/tools/ant/taskdefs/ParallelTest.java

@@ -0,0 +1,133 @@
+/*
+ * The Apache Software License, Version 1.1
+ *
+ * Copyright (c) 2002 The Apache Software Foundation.  All rights
+ * reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ *
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in
+ *    the documentation and/or other materials provided with the
+ *    distribution.
+ *
+ * 3. The end-user documentation included with the redistribution, if
+ *    any, must include the following acknowlegement:
+ *       "This product includes software developed by the
+ *        Apache Software Foundation (http://www.apache.org/)."
+ *    Alternately, this acknowlegement may appear in the software itself,
+ *    if and wherever such third-party acknowlegements normally appear.
+ *
+ * 4. The names "The Jakarta Project", "Ant", and "Apache Software
+ *    Foundation" must not be used to endorse or promote products derived
+ *    from this software without prior written permission. For written
+ *    permission, please contact apache@apache.org.
+ *
+ * 5. Products derived from this software may not be called "Apache"
+ *    nor may "Apache" appear in their names without prior written
+ *    permission of the Apache Group.
+ *
+ * THIS SOFTWARE IS PROVIDED ``AS IS'' AND ANY EXPRESSED OR IMPLIED
+ * WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+ * DISCLAIMED.  IN NO EVENT SHALL THE APACHE SOFTWARE FOUNDATION OR
+ * ITS CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
+ * USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
+ * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+ * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
+ * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ * ====================================================================
+ *
+ * This software consists of voluntary contributions made by many
+ * individuals on behalf of the Apache Software Foundation.  For more
+ * information on the Apache Software Foundation, please see
+ * <http://www.apache.org/>.
+ */
+package org.apache.tools.ant.taskdefs;
+import java.io.PrintStream;
+
+import org.apache.tools.ant.BuildFileTest;
+import org.apache.tools.ant.DemuxOutputStream;
+import org.apache.tools.ant.Project;
+
+/**
+ * Test of the parallel TaskContainer
+ *
+ * @author <a href="mailto:conor@apache.org">Conor MacNeill</a>
+ * @created 21 February 2002
+ */
+public class ParallelTest extends BuildFileTest {
+    /** Standard property value for the basic test */
+    public final static String DIRECT_MESSAGE = "direct";
+    /** Standard property value for the basic and fail test */
+    public final static String DELAYED_MESSAGE = "delayed";
+    /** Standard property value for the fail test */
+    public final static String FAILURE_MESSAGE = "failure";
+
+    /** the build fiel associated with this test */
+    public final static String TEST_BUILD_FILE
+         = "src/etc/testcases/taskdefs/parallel.xml";
+
+    /**
+     * Constructor for the ParallelTest object
+     *
+     * @param name name of the test
+     */
+    public ParallelTest(String name) {
+        super(name);
+    }
+
+    /** The JUnit setup method */
+    public void setUp() {
+        configureProject(TEST_BUILD_FILE);
+    }
+
+    /** tests basic operation of the parallel task */
+    public void testBasic() {
+        // should get no output at all
+        Project project = getProject();
+        project.setUserProperty("test.direct", DIRECT_MESSAGE);
+        project.setUserProperty("test.delayed", DELAYED_MESSAGE);
+        expectOutputAndError("testBasic", "", "");
+        String log = getLog();
+        assertEquals("parallel tasks didn't output correct data", log,
+            DIRECT_MESSAGE + DELAYED_MESSAGE);
+
+    }
+
+    /** tests the failure of a task within a parallel construction */
+    public void testFail() {
+        // should get no output at all
+        Project project = getProject();
+        project.setUserProperty("test.failure", FAILURE_MESSAGE);
+        project.setUserProperty("test.delayed", DELAYED_MESSAGE);
+        expectBuildExceptionContaining("testFail",
+            "fail task in one parallel branch", FAILURE_MESSAGE);
+    }
+
+    /** tests the demuxing of output streams in a multithreaded situation */
+    public void testDemux() {
+        Project project = getProject();
+        project.addTaskDefinition("demuxtest", DemuxOutputTask.class);
+        PrintStream out = System.out;
+        PrintStream err = System.err;
+        System.setOut(new PrintStream(new DemuxOutputStream(project, false)));
+        System.setErr(new PrintStream(new DemuxOutputStream(project, true)));
+
+        try {
+            project.executeTarget("testDemux");
+        } finally {
+            System.setOut(out);
+            System.setErr(err);
+        }
+    }
+}
+