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

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

*** 28,37 ****
--- 28,38 ----
  import java.io.ByteArrayOutputStream;
  import java.io.File;
  import java.io.FileOutputStream;
  import java.io.IOException;
  import java.io.InputStream;
+ import java.io.ObjectOutputStream;
  import java.io.OutputStream;
  import java.io.PrintStream;
  import java.io.PrintWriter;
  import java.io.Serializable;
  import java.lang.Process;
*** 132,141 ****
--- 133,192 ----
   * @author      Ann Wollrath
   * @since       1.2
   */
  public class Activation implements Serializable {
  
+     /*
+      * Synchronization occurs over several objects in this code.
+      * These objects are:
+      *
+      *  - startupLock
+      *  - initLock
+      *  - Activation.this
+      *  - instances of GroupEntry
+      *  - log (a ReliableLog)
+      *  - idTable (a HashMap)
+      *  - groupTable (a HashMap)
+      *
+      * The startupLock and initLock locks are used only during
+      * initialization and shutdown. The lock on Activation.this
+      * is used to guard groupSemaphore and to guard writes to the
+      * shuttingDown flag (which is volatile and which is read
+      * without synchronization).
+      *
+      * Several operations on GroupEntry instances hold a lock on
+      * that instance while performing logging operations and while
+      * while manipulating idTable or groupTable.
+      *
+      * Log operations, notably addLogRecord(), hold a lock on the
+      * log instance. (Methods of the log itself also take locks
+      * on the log object.) The log.snapshot() operation serializes
+      * the log, including instances of Activation (this class),
+      * which in turn must lock idTable and groupTable to prevent
+      * concurrent modification during serialization.
+      *
+      * The idTable and groupTable HashMaps are locked while simple
+      * operations are performed on them. Note that we cannot
+      * straightforwardly convert these to ConcurrentHashMap,
+      * because it has a different serialized form from HashMap.
+      *
+      * During serialization, the writeObject() method of this class
+      * takes locks on groupTable and idTable in that order.
+      * 
+      * Based on the above paths through the code, a partial
+      * ordering for the above locks must be as follows:
+      *
+      * 1. GroupEntry instances
+      * 2. log
+      * 3. groupTable
+      * 4. idTable
+      *
+      * For example, code may lock the log while holding the
+      * lock for a GroupEntry, but code must not lock the log
+      * while holding a lock on groupTable.
+      */
+ 
      /** indicate compatibility with JDK 1.2 version of class */
      private static final long serialVersionUID = 2921265612698155191L;
  
      private static final byte MAJOR_VERSION = 1;
      private static final byte MINOR_VERSION = 0;
*** 272,281 ****
--- 323,345 ----
                  e.printStackTrace();
              }
          }
      }
  
+     private void writeObject(ObjectOutputStream ostream) throws IOException {
+         // Synchronize on groupTable and idTable while writing them to the
+         // object stream in order to avoid ConcurrentModificationException. It
+         // might be preferable to synchronize them individually, but then we'd
+         // have to write the fields individually instead of using
+         // defaultWriteObject().
+         synchronized (groupTable) {
+             synchronized (idTable) {
+                 ostream.defaultWriteObject();
+             }
+         }
+     }
+ 
      private static class SystemRegistryImpl extends RegistryImpl {
  
          private static final String NAME = ActivationSystem.class.getName();
          private final ActivationSystem systemStub;
  
*** 513,527 ****
              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);
              }
          }
  
          public ActivationDesc setActivationDesc(ActivationID id,
                                                  ActivationDesc desc)
              throws ActivationException, UnknownObjectException, RemoteException
--- 577,592 ----
              checkShutdown();
              RegistryImpl.checkAccess("ActivationSystem.unregisterGroup");
  
              // remove entry before unregister so state is updated before
              // logged
+             GroupEntry entry;
              synchronized (groupTable) {
!                 entry = getGroupEntry(id);
                  groupTable.remove(id);
              }
+             entry.unregisterGroup(true);
          }
  
          public ActivationDesc setActivationDesc(ActivationID id,
                                                  ActivationDesc desc)
              throws ActivationException, UnknownObjectException, RemoteException