src/share/classes/sun/rmi/server/Activation.java

Print this page
rev 3756 : 6896297: (rmi) fix ConcurrentModificationException causing TCK failure
Reviewed-by: XXX

@@ -28,10 +28,11 @@
 import java.io.ByteArrayOutputStream;
 import java.io.File;
 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;
 import java.io.Serializable;
 import java.lang.Process;

@@ -96,10 +97,11 @@
 import java.util.Map;
 import java.util.MissingResourceException;
 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;
 import sun.rmi.runtime.NewThreadAction;
 import sun.rmi.server.UnicastServerRef;

@@ -145,14 +147,14 @@
     private static Method execPolicyMethod;
     private static boolean debugExec;
 
     /** 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;
 
     /** number of simultaneous group exec's */

@@ -235,12 +237,11 @@
         shutdownHook =  new ShutdownHook();
         groupSemaphore = getInt("sun.rmi.activation.groupThrottle", 3);
         groupCounter = 0;
         Runtime.getRuntime().addShutdownHook(shutdownHook);
         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
             // (via checkShutdown) to make sure they don't happen in the
             // middle of this block.  This block must not cause any such

@@ -272,10 +273,27 @@
                 e.printStackTrace();
             }
         }
     }
 
+    /**
+     * 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();
         private final ActivationSystem systemStub;
 

@@ -486,13 +504,11 @@
             checkArgs(desc, null);
 
             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);
-            }
             addLogRecord(new LogRegisterGroup(id, desc));
             return id;
         }
 
         public ActivationMonitor activeGroup(ActivationGroupID id,

@@ -513,15 +529,11 @@
             checkShutdown();
             RegistryImpl.checkAccess("ActivationSystem.unregisterGroup");
 
             // 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,
                                                 ActivationDesc desc)
             throws ActivationException, UnknownObjectException, RemoteException

@@ -635,16 +647,11 @@
                  */
                 unexport(activator);
                 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();
                 }
 
                 Runtime.getRuntime().removeShutdownHook(shutdownHook);
 

@@ -691,67 +698,84 @@
             synchronized (Activation.this) {
                 shuttingDown = true;
             }
 
             // destroy all child processes (groups) quickly
-            synchronized (groupTable) {
                 for (GroupEntry groupEntry : groupTable.values()) {
                     groupEntry.shutdownFast();
                 }
             }
         }
-    }
 
     /**
      * Returns the groupID for a given id of an object in the group.
      * Throws UnknownObjectException if the object is not registered.
      */
     private ActivationGroupID getGroupID(ActivationID id)
         throws UnknownObjectException
     {
-        synchronized (idTable) {
             ActivationGroupID groupID = idTable.get(id);
             if (groupID != null) {
                 return groupID;
             }
-        }
         throw new UnknownObjectException("unknown object: " + id);
     }
 
     /**
-     * Returns the group entry for the group id. Throws
-     * UnknownGroupException if the group is not registered.
+     * Returns the group entry for the group id, optionally removing it.
+     * Throws UnknownGroupException if the group is not registered.
      */
-    private GroupEntry getGroupEntry(ActivationGroupID id)
+    private GroupEntry getGroupEntry(ActivationGroupID id, boolean rm)
         throws UnknownGroupException
     {
         if (id.getClass() == ActivationGroupID.class) {
-            synchronized (groupTable) {
-                GroupEntry entry = groupTable.get(id);
+            GroupEntry entry;
+            if (rm) {
+                entry = groupTable.remove(id);
+            } else {
+                entry = groupTable.get(id);
+            }
                 if (entry != null && !entry.removed) {
                     return entry;
                 }
             }
-        }
         throw new UnknownGroupException("group unknown");
     }
 
     /**
+     * Returns the group entry for the group id. Throws
+     * UnknownGroupException if the group is not registered.
+     */
+    private GroupEntry getGroupEntry(ActivationGroupID id)
+        throws UnknownGroupException
+    {
+        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);
+    }
+
+    /**
      * Returns the group entry for the object's id. Throws
      * UnknownObjectException if the object is not registered or the
      * object's group is not registered.
      */
     private GroupEntry getGroupEntry(ActivationID id)
         throws UnknownObjectException
     {
         ActivationGroupID gid = getGroupID(id);
-        synchronized (groupTable) {
             GroupEntry entry = groupTable.get(gid);
-            if (entry != null) {
+        if (entry != null && !entry.removed) {
                 return entry;
             }
-        }
         throw new UnknownObjectException("object's group removed");
     }
 
     /**
      * Container for group information: group's descriptor, group's

@@ -880,13 +904,11 @@
             if (desc.getRestartMode() == true) {
                 restartSet.add(id);
             }
 
             // table insertion must take place before log update
-            synchronized (idTable) {
                 idTable.put(id, groupID);
-            }
 
             if (addRecord) {
                 addLogRecord(new LogRegisterObject(id, desc));
             }
         }

@@ -899,14 +921,12 @@
             objects.remove(id);
             if (objEntry.desc.getRestartMode() == true) {
                 restartSet.remove(id);
             }
 
-            // table insertion must take place before log update
-            synchronized (idTable) {
+            // table removal must take place before log update
                 idTable.remove(id);
-            }
             if (addRecord) {
                 addLogRecord(new LogUnregisterObject(id));
             }
         }
 

@@ -917,13 +937,11 @@
             removed = true;
             for (Map.Entry<ActivationID,ObjectEntry> entry :
                      objects.entrySet())
             {
                 ActivationID id = entry.getKey();
-                synchronized (idTable) {
                     idTable.remove(id);
-                }
                 ObjectEntry objEntry = entry.getValue();
                 objEntry.removed = true;
             }
             objects.clear();
             restartSet.clear();