[JENKINS-38520] Every message in agent log appears twice with the `-a…

…gentLog` option (#664)

src/main/java/hudson/remoting/TeeOutputStream.java

@@ -25,81 +25,99 @@
 import java.io.OutputStream;
 
 /**
- * Classic splitter of OutputStream. Named after the unix 'tee'
- * command. It allows a stream to be branched off so there
- * are now two streams.
- *
- * @version $Id: TeeOutputStream.java 610010 2008-01-08 14:50:59Z niallp $
+ * Classic splitter of {@link OutputStream}. Named after the Unix 'tee' command. It allows a stream
+ * to be branched off so there are now two streams.
  */
 @Restricted(NoExternalUse.class)
 public class TeeOutputStream extends FilterOutputStream {
 
-    /** the second OutputStream to write to */
+    /**
+     * The second OutputStream to write to.
+     *
+     * <p>TODO Make private and final in 3.0.
+     */
     protected OutputStream branch;
 
     /**
      * Constructs a TeeOutputStream.
+     *
      * @param out the main OutputStream
      * @param branch the second OutputStream
      */
-    public TeeOutputStream( OutputStream out, OutputStream branch ) {
+    public TeeOutputStream(final OutputStream out, final OutputStream branch) {
         super(out);
         this.branch = branch;
     }
 
     /**
-     * Write the bytes to both streams.
+     * Writes the bytes to both streams.
+     *
      * @param b the bytes to write
-     * @throws IOException if an I/O error occurs
+     * @throws IOException if an I/O error occurs.
      */
     @Override
-    public synchronized void write(@NonNull byte[] b) throws IOException {
-        super.write(b);
+    public synchronized void write(@NonNull final byte[] b) throws IOException {
+        out.write(b);
         this.branch.write(b);
     }
 
     /**
-     * Write the specified bytes to both streams.
+     * Writes the specified bytes to both streams.
+     *
      * @param b the bytes to write
      * @param off The start offset
      * @param len The number of bytes to write
-     * @throws IOException if an I/O error occurs
+     * @throws IOException if an I/O error occurs.
      */
     @Override
-    public synchronized void write(@NonNull byte[] b, int off, int len) throws IOException {
-        super.write(b, off, len);
+    public synchronized void write(@NonNull final byte[] b, final int off, final int len) throws IOException {
+        out.write(b, off, len);
         this.branch.write(b, off, len);
     }
 
     /**
-     * Write a byte to both streams.
+     * Writes a byte to both streams.
+     *
      * @param b the byte to write
-     * @throws IOException if an I/O error occurs
+     * @throws IOException if an I/O error occurs.
      */
     @Override
-    public synchronized void write(int b) throws IOException {
-        super.write(b);
+    public synchronized void write(final int b) throws IOException {
+        out.write(b);
         this.branch.write(b);
     }
 
     /**
      * Flushes both streams.
-     * @throws IOException if an I/O error occurs
+     *
+     * @throws IOException if an I/O error occurs.
      */
     @Override
     public void flush() throws IOException {
-        super.flush();
+        out.flush();
         this.branch.flush();
     }
 
     /**
-     * Closes both streams.
-     * @throws IOException if an I/O error occurs
+     * Closes both output streams.
+     *
+     * <p>If closing the main output stream throws an exception, attempt to close the branch output
+     * stream.
+     *
+     * <p>If closing the main and branch output streams both throw exceptions, which exceptions is
+     * thrown by this method is currently unspecified and subject to change.
+     *
+     * @throws IOException if an I/O error occurs.
      */
     @Override
     public void close() throws IOException {
-        super.close();
-        this.branch.close();
+        try {
+            if (out != null) {
+                out.close();
+            }
+        } finally {
+            this.branch.close();
+        }
     }
 
 }

src/test/java/hudson/remoting/TeeOutputStreamTest.java

@@ -0,0 +1,140 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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 hudson.remoting;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.fail;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+
+import java.io.ByteArrayOutputStream;
+import java.io.FilterOutputStream;
+import java.io.IOException;
+import java.io.OutputStream;
+import org.junit.jupiter.api.Test;
+
+/**
+ * Tests {@link TeeOutputStream}.
+ */
+public class TeeOutputStreamTest {
+
+    private void assertByteArrayEquals(final String msg, final byte[] array1, final byte[] array2) {
+        assertEquals(array1.length, array2.length, msg + ": array size mismatch");
+        for (int i = 0; i < array1.length; i++) {
+            assertEquals(array1[i], array2[i], msg + ": array[ " + i + "] mismatch");
+        }
+    }
+
+    /**
+     * Tests that the main {@code OutputStream} is closed when closing the branch {@code OutputStream} throws an
+     * exception on {@link TeeOutputStream#close()}.
+     */
+    @Test
+    public void testIOExceptionOnClose() throws IOException {
+        final OutputStream badOs = new ThrowOnCloseOutputStream();
+        final ByteArrayOutputStream goodOs = mock(ByteArrayOutputStream.class);
+        final TeeOutputStream tos = new TeeOutputStream(badOs, goodOs);
+        try {
+            tos.close();
+            fail("Expected " + IOException.class.getName());
+        } catch (final IOException e) {
+            verify(goodOs).close();
+        }
+    }
+
+    /**
+     * Tests that the branch {@code OutputStream} is closed when closing the main {@code OutputStream} throws an
+     * exception on {@link TeeOutputStream#close()}.
+     */
+    @Test
+    public void testIOExceptionOnCloseBranch() throws IOException {
+        final OutputStream badOs = new ThrowOnCloseOutputStream();
+        final ByteArrayOutputStream goodOs = mock(ByteArrayOutputStream.class);
+        final TeeOutputStream tos = new TeeOutputStream(goodOs, badOs);
+        try {
+            tos.close();
+            fail("Expected " + IOException.class.getName());
+        } catch (final IOException e) {
+            verify(goodOs).close();
+        }
+    }
+
+    @Test
+    public void testTee() throws IOException {
+        final ByteArrayOutputStream baos1 = new ByteArrayOutputStream();
+        final ByteArrayOutputStream baos2 = new ByteArrayOutputStream();
+        final ByteArrayOutputStream expected = new ByteArrayOutputStream();
+
+        try (TeeOutputStream tos = new TeeOutputStream(baos1, baos2)) {
+            for (int i = 0; i < 20; i++) {
+                tos.write(i);
+                expected.write(i);
+            }
+            assertByteArrayEquals("TeeOutputStream.write(int)", expected.toByteArray(), baos1.toByteArray());
+            assertByteArrayEquals("TeeOutputStream.write(int)", expected.toByteArray(), baos2.toByteArray());
+
+            final byte[] array = new byte[10];
+            for (int i = 20; i < 30; i++) {
+                array[i - 20] = (byte) i;
+            }
+            tos.write(array);
+            expected.write(array);
+            assertByteArrayEquals("TeeOutputStream.write(byte[])", expected.toByteArray(), baos1.toByteArray());
+            assertByteArrayEquals("TeeOutputStream.write(byte[])", expected.toByteArray(), baos2.toByteArray());
+
+            for (int i = 25; i < 35; i++) {
+                array[i - 25] = (byte) i;
+            }
+            tos.write(array, 5, 5);
+            expected.write(array, 5, 5);
+            assertByteArrayEquals(
+                    "TeeOutputStream.write(byte[], int, int)", expected.toByteArray(), baos1.toByteArray());
+            assertByteArrayEquals(
+                    "TeeOutputStream.write(byte[], int, int)", expected.toByteArray(), baos2.toByteArray());
+
+            expected.flush();
+            expected.close();
+
+            tos.flush();
+        }
+    }
+
+    static class ThrowOnCloseOutputStream extends FilterOutputStream {
+
+        /**
+         * Default constructor.
+         */
+        public ThrowOnCloseOutputStream() {
+            super(OutputStream.nullOutputStream());
+        }
+
+        /**
+         * @param proxy OutputStream to delegate to.
+         */
+        public ThrowOnCloseOutputStream(final OutputStream proxy) {
+            super(proxy);
+        }
+
+        /**
+         * @see java.io.OutputStream#close()
+         */
+        @Override
+        public void close() throws IOException {
+            throw new IOException(getClass().getSimpleName() + ".close() called.");
+        }
+    }
+}