6927486: Hashtable writeObject() may deadlock
Summary: Do not synchronize on self while writing hash table elements
Reviewed-by: alanb, mduigou
Contributed-by: Neil Richards <neil.richards@ngmr.net>
--- a/jdk/src/share/classes/java/util/Hashtable.java Mon Feb 14 10:48:46 2011 -0800
+++ b/jdk/src/share/classes/java/util/Hashtable.java Mon Feb 14 11:00:02 2011 -0800
@@ -845,24 +845,36 @@
* for each key-value mapping represented by the Hashtable
* The key-value mappings are emitted in no particular order.
*/
- private synchronized void writeObject(java.io.ObjectOutputStream s)
- throws IOException
- {
- // Write out the length, threshold, loadfactor
- s.defaultWriteObject();
+ private void writeObject(java.io.ObjectOutputStream s)
+ throws IOException {
+ Entry<Object, Object> entryStack = null;
+
+ synchronized (this) {
+ // Write out the length, threshold, loadfactor
+ s.defaultWriteObject();
+
+ // Write out length, count of elements
+ s.writeInt(table.length);
+ s.writeInt(count);
- // Write out length, count of elements and then the key/value objects
- s.writeInt(table.length);
- s.writeInt(count);
- for (int index = table.length-1; index >= 0; index--) {
- Entry entry = table[index];
+ // Stack copies of the entries in the table
+ for (int index = 0; index < table.length; index++) {
+ Entry entry = table[index];
- while (entry != null) {
- s.writeObject(entry.key);
- s.writeObject(entry.value);
- entry = entry.next;
+ while (entry != null) {
+ entryStack =
+ new Entry<>(0, entry.key, entry.value, entryStack);
+ entry = entry.next;
+ }
}
}
+
+ // Write out the key/value objects from the stacked entries
+ while (entryStack != null) {
+ s.writeObject(entryStack.key);
+ s.writeObject(entryStack.value);
+ entryStack = entryStack.next;
+ }
}
/**
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/jdk/test/java/util/Hashtable/SerializationDeadlock.java Mon Feb 14 11:00:02 2011 -0800
@@ -0,0 +1,156 @@
+/*
+ * Copyright (c) 2010, 2011 Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ *
+ * -------------------------------------------
+ *
+ * Portions Copyright (c) 2010, 2011 IBM Corporation
+ */
+
+/*
+ * @test
+ * @bug 6927486
+ * @summary Serializing Hashtable objects which refer to each other should not be able to deadlock.
+ * @author Neil Richards <neil.richards@ngmr.net>, <neil_richards@uk.ibm.com>
+ */
+
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.ObjectOutputStream;
+import java.io.PrintWriter;
+import java.io.Serializable;
+import java.io.StringWriter;
+import java.util.ArrayList;
+import java.util.Hashtable;
+import java.util.List;
+import java.util.concurrent.CyclicBarrier;
+
+public class SerializationDeadlock {
+ public static void main(final String[] args) throws Exception {
+ // Test for Hashtable serialization deadlock
+ final Hashtable<Object, Object> h1 = new Hashtable<>();
+ final Hashtable<Object, Object> h2 = new Hashtable<>();
+ final TestBarrier testStart = new TestBarrier(3);
+
+ // Populate the hashtables so that they refer to each other
+ h1.put(testStart, h2);
+ h2.put(testStart, h1);
+
+ final CyclicBarrier testEnd = new CyclicBarrier(3);
+ final TestThread t1 = new TestThread(h1, testEnd);
+ final TestThread t2 = new TestThread(h2, testEnd);
+
+ t1.start();
+ t2.start();
+
+ // Wait for both test threads to have initiated serialization
+ // of the 'testStart' object (and hence of both 'h1' and 'h2')
+ testStart.await();
+
+ // Wait for both test threads to successfully finish serialization
+ // of 'h1' and 'h2'.
+ System.out.println("Waiting for Hashtable serialization to complete ...");
+ System.out.println("(This test will hang if serialization deadlocks)");
+ testEnd.await();
+ System.out.println("Test PASSED: serialization completed successfully");
+
+ 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 test threads 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 List<Exception> exceptions = new ArrayList<>();
+
+ private final Hashtable<Object, Object> hashtable;
+ private final CyclicBarrier testEnd;
+
+ public TestThread(final Hashtable<Object, Object> hashtable,
+ final CyclicBarrier testEnd) {
+ this.hashtable = hashtable;
+ this.testEnd = testEnd;
+ setDaemon(true);
+ }
+
+ public void run() {
+ try {
+ final ByteArrayOutputStream baos = new ByteArrayOutputStream();
+ final ObjectOutputStream oos = new ObjectOutputStream(baos);
+
+ oos.writeObject(hashtable);
+ 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()) {
+ throw new RuntimeException(getErrorText(exceptions));
+ }
+ }
+
+ private static String getErrorText(final List<Exception> exceptions) {
+ final StringWriter sw = new StringWriter();
+ final PrintWriter pw = new PrintWriter(sw);
+
+ pw.println("Test ERROR: Unexpected exceptions thrown on test threads:");
+ for (Exception exception : exceptions) {
+ pw.print("\t");
+ pw.println(exception);
+ for (StackTraceElement element : exception.getStackTrace()) {
+ pw.print("\t\tat ");
+ pw.println(element);
+ }
+ }
+
+ pw.close();
+ return sw.toString();
+ }
+ }
+}
+
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/jdk/test/java/util/Hashtable/SimpleSerialization.java Mon Feb 14 11:00:02 2011 -0800
@@ -0,0 +1,86 @@
+/*
+ * Copyright (c) 2010, 2011 Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ *
+ * -------------------------------------------
+ *
+ * Portions Copyright (c) 2010, 2011 IBM Corporation
+ */
+
+/*
+ * @test
+ * @bug 6927486
+ * @summary A serialized Hashtable can be de-serialized properly.
+ * @author Neil Richards <neil.richards@ngmr.net>, <neil_richards@uk.ibm.com>
+ */
+
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.ObjectInputStream;
+import java.io.ObjectOutputStream;
+import java.io.PrintWriter;
+import java.io.StringWriter;
+import java.util.Hashtable;
+
+public class SimpleSerialization {
+ public static void main(final String[] args) throws Exception {
+ Hashtable<String, String> h1 = new Hashtable<>();
+
+ h1.put("key", "value");
+
+ final ByteArrayOutputStream baos = new ByteArrayOutputStream();
+ final ObjectOutputStream oos = new ObjectOutputStream(baos);
+
+ oos.writeObject(h1);
+ oos.close();
+
+ final byte[] data = baos.toByteArray();
+ final ByteArrayInputStream bais = new ByteArrayInputStream(data);
+ final ObjectInputStream ois = new ObjectInputStream(bais);
+
+ final Object deserializedObject = ois.readObject();
+ ois.close();
+
+ if (false == h1.equals(deserializedObject)) {
+ throw new RuntimeException(getFailureText(h1, deserializedObject));
+ }
+ }
+
+ private static String getFailureText(final Object orig, final Object copy) {
+ final StringWriter sw = new StringWriter();
+ final PrintWriter pw = new PrintWriter(sw);
+
+ pw.println("Test FAILED: Deserialized object is not equal to the original object");
+ pw.print("\tOriginal: ");
+ printObject(pw, orig).println();
+ pw.print("\tCopy: ");
+ printObject(pw, copy).println();
+
+ pw.close();
+ return sw.toString();
+ }
+
+ private static PrintWriter printObject(final PrintWriter pw, final Object o) {
+ pw.printf("%s@%08x", o.getClass().getName(), System.identityHashCode(o));
+ return pw;
+ }
+}