6896297: (rmi) fix ConcurrentModificationException causing TCK failure
Reviewed-by: alanb, dholmes, peterjones
--- a/jdk/src/share/classes/sun/rmi/log/ReliableLog.java Wed Apr 20 12:05:17 2011 +0100
+++ b/jdk/src/share/classes/sun/rmi/log/ReliableLog.java Wed Apr 20 16:30:38 2011 -0700
@@ -380,9 +380,7 @@
} catch (IOException e) {
throw e;
} catch (Exception e) {
- throw new IOException("snapshot failed with exception of type: " +
- e.getClass().getName() +
- ", message was: " + e.getMessage());
+ throw new IOException("snapshot failed", e);
}
lastSnapshot = System.currentTimeMillis();
} finally {
--- a/jdk/src/share/classes/sun/rmi/server/Activation.java Wed Apr 20 12:05:17 2011 +0100
+++ b/jdk/src/share/classes/sun/rmi/server/Activation.java Wed Apr 20 16:30:38 2011 -0700
@@ -30,6 +30,7 @@
import java.io.FileOutputStream;
import java.io.IOException;
import java.io.InputStream;
+import java.io.ObjectInputStream;
import java.io.OutputStream;
import java.io.PrintStream;
import java.io.PrintWriter;
@@ -98,6 +99,7 @@
import java.util.Properties;
import java.util.ResourceBundle;
import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
import sun.rmi.log.LogHandler;
import sun.rmi.log.ReliableLog;
import sun.rmi.registry.RegistryImpl;
@@ -147,10 +149,10 @@
/** maps activation id to its respective group id */
private Map<ActivationID,ActivationGroupID> idTable =
- new HashMap<ActivationID,ActivationGroupID>();
+ new ConcurrentHashMap<>();
/** maps group id to its GroupEntry groups */
private Map<ActivationGroupID,GroupEntry> groupTable =
- new HashMap<ActivationGroupID,GroupEntry>();
+ new ConcurrentHashMap<>();
private byte majorVersion = MAJOR_VERSION;
private byte minorVersion = MINOR_VERSION;
@@ -236,9 +238,11 @@
groupSemaphore = getInt("sun.rmi.activation.groupThrottle", 3);
groupCounter = 0;
Runtime.getRuntime().addShutdownHook(shutdownHook);
+
+ // Use array size of 0, since the value from calling size()
+ // may be out of date by the time toArray() is called.
ActivationGroupID[] gids =
- groupTable.keySet().toArray(
- new ActivationGroupID[groupTable.size()]);
+ groupTable.keySet().toArray(new ActivationGroupID[0]);
synchronized (startupLock = new Object()) {
// all the remote methods briefly synchronize on startupLock
@@ -274,6 +278,23 @@
}
}
+ /**
+ * Previous versions used HashMap instead of ConcurrentHashMap.
+ * Replace any HashMaps found during deserialization with
+ * ConcurrentHashMaps.
+ */
+ private void readObject(ObjectInputStream ois)
+ throws IOException, ClassNotFoundException
+ {
+ ois.defaultReadObject();
+ if (! (groupTable instanceof ConcurrentHashMap)) {
+ groupTable = new ConcurrentHashMap<>(groupTable);
+ }
+ if (! (idTable instanceof ConcurrentHashMap)) {
+ idTable = new ConcurrentHashMap<>(idTable);
+ }
+ }
+
private static class SystemRegistryImpl extends RegistryImpl {
private static final String NAME = ActivationSystem.class.getName();
@@ -488,9 +509,7 @@
ActivationGroupID id = new ActivationGroupID(systemStub);
GroupEntry entry = new GroupEntry(id, desc);
// table insertion must take place before log update
- synchronized (groupTable) {
- groupTable.put(id, entry);
- }
+ groupTable.put(id, entry);
addLogRecord(new LogRegisterGroup(id, desc));
return id;
}
@@ -515,11 +534,7 @@
// remove entry before unregister so state is updated before
// logged
- synchronized (groupTable) {
- GroupEntry entry = getGroupEntry(id);
- groupTable.remove(id);
- entry.unregisterGroup(true);
- }
+ removeGroupEntry(id).unregisterGroup(true);
}
public ActivationDesc setActivationDesc(ActivationID id,
@@ -637,12 +652,7 @@
unexport(system);
// destroy all child processes (groups)
- GroupEntry[] groupEntries;
- synchronized (groupTable) {
- groupEntries = groupTable.values().
- toArray(new GroupEntry[groupTable.size()]);
- }
- for (GroupEntry groupEntry : groupEntries) {
+ for (GroupEntry groupEntry : groupTable.values()) {
groupEntry.shutdown();
}
@@ -693,10 +703,8 @@
}
// destroy all child processes (groups) quickly
- synchronized (groupTable) {
- for (GroupEntry groupEntry : groupTable.values()) {
- groupEntry.shutdownFast();
- }
+ for (GroupEntry groupEntry : groupTable.values()) {
+ groupEntry.shutdownFast();
}
}
}
@@ -708,13 +716,32 @@
private ActivationGroupID getGroupID(ActivationID id)
throws UnknownObjectException
{
- synchronized (idTable) {
- ActivationGroupID groupID = idTable.get(id);
- if (groupID != null) {
- return groupID;
+ ActivationGroupID groupID = idTable.get(id);
+ if (groupID != null) {
+ return groupID;
+ }
+ throw new UnknownObjectException("unknown object: " + id);
+ }
+
+ /**
+ * Returns the group entry for the group id, optionally removing it.
+ * Throws UnknownGroupException if the group is not registered.
+ */
+ private GroupEntry getGroupEntry(ActivationGroupID id, boolean rm)
+ throws UnknownGroupException
+ {
+ if (id.getClass() == ActivationGroupID.class) {
+ GroupEntry entry;
+ if (rm) {
+ entry = groupTable.remove(id);
+ } else {
+ entry = groupTable.get(id);
+ }
+ if (entry != null && !entry.removed) {
+ return entry;
}
}
- throw new UnknownObjectException("unknown object: " + id);
+ throw new UnknownGroupException("group unknown");
}
/**
@@ -724,15 +751,17 @@
private GroupEntry getGroupEntry(ActivationGroupID id)
throws UnknownGroupException
{
- if (id.getClass() == ActivationGroupID.class) {
- synchronized (groupTable) {
- GroupEntry entry = groupTable.get(id);
- if (entry != null && !entry.removed) {
- return entry;
- }
- }
- }
- throw new UnknownGroupException("group unknown");
+ return getGroupEntry(id, false);
+ }
+
+ /**
+ * Removes and returns the group entry for the group id. Throws
+ * UnknownGroupException if the group is not registered.
+ */
+ private GroupEntry removeGroupEntry(ActivationGroupID id)
+ throws UnknownGroupException
+ {
+ return getGroupEntry(id, true);
}
/**
@@ -744,11 +773,9 @@
throws UnknownObjectException
{
ActivationGroupID gid = getGroupID(id);
- synchronized (groupTable) {
- GroupEntry entry = groupTable.get(gid);
- if (entry != null) {
- return entry;
- }
+ GroupEntry entry = groupTable.get(gid);
+ if (entry != null && !entry.removed) {
+ return entry;
}
throw new UnknownObjectException("object's group removed");
}
@@ -882,9 +909,7 @@
}
// table insertion must take place before log update
- synchronized (idTable) {
- idTable.put(id, groupID);
- }
+ idTable.put(id, groupID);
if (addRecord) {
addLogRecord(new LogRegisterObject(id, desc));
@@ -901,10 +926,8 @@
restartSet.remove(id);
}
- // table insertion must take place before log update
- synchronized (idTable) {
- idTable.remove(id);
- }
+ // table removal must take place before log update
+ idTable.remove(id);
if (addRecord) {
addLogRecord(new LogUnregisterObject(id));
}
@@ -919,9 +942,7 @@
objects.entrySet())
{
ActivationID id = entry.getKey();
- synchronized (idTable) {
- idTable.remove(id);
- }
+ idTable.remove(id);
ObjectEntry objEntry = entry.getValue();
objEntry.removed = true;
}