Sunbug 6934356: Vector.writeObject() synchronization risks serialization deadlock

[Neil Richards]

Hello.

I have a fix and testcase for problem 6934356 in the Java bug database
- "Vector.writeObject() synchronization risks serialization deadlock".
I've included the 'hg diff -g' output below.

I'm new to OpenJDK - though not to Java SE implementation development
- so hope that this is correct mailing list to ask for the code to be
reviewed.
(Please point me towards a better list if this is not so).

Also, given that this is a reported bug in the Java bug database, I'm
a little confused as to whether I need to additionally raise it in
https://bugs.openjdk.java.net/

And, indeed, whether it is better for me to include the 'hg diff -g'
output inline, as I have done below, or as an attachment, or in some
other fashion.

Basically, any guidance will be gratefully received :-)

Thanks,
Neil

--
Unless stated above:
IBM United Kingdom Limited - Registered in England and Wales with number 741598.
Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU
--

diff --git a/src/share/classes/java/util/Vector.java
b/src/share/classes/java/util/Vector.java
--- a/src/share/classes/java/util/Vector.java
+++ b/src/share/classes/java/util/Vector.java
@@ -1053,10 +1053,20 @@
      * is, serialize it).  This method is present merely for synchronization.
      * It just calls the default writeObject method.
      */
-    private synchronized void writeObject(java.io.ObjectOutputStream s)
+    private void writeObject(java.io.ObjectOutputStream s)
         throws java.io.IOException
     {
-        s.defaultWriteObject();
+        final java.io.ObjectOutputStream.PutField fields = s.putFields();
+        Object[] data = null;
+        synchronized (this) {
+            fields.put("capacityIncrement", capacityIncrement);
+            fields.put("elementCount", elementCount);
+            final int dataLength = elementData.length;
+            data = new Object[dataLength];
+            System.arraycopy(elementData, 0, data, 0, dataLength);
+        }
+        fields.put("elementData", data);
+        s.writeFields();
     }

     /**
diff --git a/test/java/util/Vector/SerializationDeadlock.java
b/test/java/util/Vector/SerializationDeadlock.java
new file mode 100644
--- /dev/null
+++ b/test/java/util/Vector/SerializationDeadlock.java
@@ -0,0 +1,111 @@
+/*
+ * @test
+ * @bug 6934356
+ * @summary Serializing Vector objects which refer to each other
should not be able to deadlock.
+ * @author Neil Richards <neil.richards at ngmr.net>
+ */
+
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.ObjectOutputStream;
+import java.io.Serializable;
+import java.util.ArrayList;
+import java.util.Vector;
+import java.util.concurrent.CyclicBarrier;
+import java.util.concurrent.TimeoutException;
+import java.util.concurrent.TimeUnit;
+
+public class SerializationDeadlock {
+    public static void main(final String[] args) {
+        // Test for Vector serialization deadlock
+        final Vector<Object> v1 = new Vector<Object>();
+        final Vector<Object> v2 = new Vector<Object>();
+        final TestBarrier testStart = new TestBarrier(3);
+
+        v1.add(testStart);
+        v1.add(v2);
+        v2.add(testStart);
+        v2.add(v1);
+
+	final CyclicBarrier testEnd = new CyclicBarrier(3);
+        final TestThread t1 = new TestThread(v1, testEnd);
+        final TestThread t2 = new TestThread(v2, testEnd);
+
+	t1.start();
+	t2.start();
+
+	try {
+	    testStart.await();
+	    testEnd.await(1, TimeUnit.SECONDS);
+	} catch (TimeoutException te) {
+	    throw new RuntimeException("Test FAILED: Probable serialization
deadlock detected");
+	} catch (Exception e) {
+	    throw new RuntimeException("Test ERROR: Unexpected exception caught", e);
+	}
+
+	TestThread.handleExceptions();
+    }
+
+    static final class TestBarrier extends CyclicBarrier
+    implements Serializable {
+        public TestBarrier(final int count) {
+            super(count);
+        }
+
+        private void writeObject(final ObjectOutputStream oos)
+        throws IOException {
+            oos.defaultWriteObject();
+            // Wait until all TestThreads have started serializing data
+            try {
+                await();
+            } catch (final Exception e) {
+                throw new IOException("Test ERROR: Unexpected
exception caught", e);
+            }
+        }
+    }
+
+    static final class TestThread extends Thread {
+	private static final ArrayList<Exception> exceptions = new
ArrayList<Exception>();
+
+        private final Vector vector;
+        private final CyclicBarrier testEnd;
+
+        public TestThread(final Vector vector, final CyclicBarrier testEnd) {
+            this.vector = vector;
+            this.testEnd = testEnd;
+            setDaemon(true);
+        }
+
+        public void run() {
+            try {
+                final ByteArrayOutputStream baos = new ByteArrayOutputStream();
+                final ObjectOutputStream oos = new ObjectOutputStream(baos);
+
+                oos.writeObject(vector);
+                oos.close();
+            } catch (final IOException ioe) {
+                addException(ioe);
+            } finally {
+		try {
+		    testEnd.await();
+		} catch (Exception e) {
+		    addException(e);
+		}
+            }
+        }
+
+        private static synchronized void addException(final Exception
exception) {
+            exceptions.add(exception);
+        }
+
+        public static synchronized void handleExceptions() {
+            if (false == exceptions.isEmpty()) {
+		for (Exception exception : exceptions) {
+		    exception.printStackTrace();
+		}
+		throw new RuntimeException("Test ERROR: Unexpected exceptions
thrown on test threads - see stderr for details");
+	    }
+        }
+    }
+}
+