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.richa...@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"); + } + } + } +} +
