Cleanup of thread-safety

javatools/src/main/java/org/xvm/asm/Component.java

@@ -833,37 +833,40 @@ public synchronized Map<String, Component> ensureChildByNameMap()
     /**
      * Make sure that any deferred child deserialization is complete
      */
-     protected void ensureChildren()
-        {
-        if (m_abChildren != null)
-            {
-            synchronized (this)
-                {
-                byte[] ab = m_abChildren;
-                if (ab != null)
-                    {
-                    // first grab the deferred deserialization bytes and then make sure neither this nor any
-                    // sibling retains hold of it (since it indicates that deserialization is deferred)
-                    for (Iterator<Component> siblings = siblings(); siblings.hasNext(); )
-                        {
-                        siblings.next().m_abChildren = null;
+    protected void ensureChildren() {
+        for (byte[] ab = m_abChildren; ab != null; ab = m_abChildren) {
+            synchronized (ab) { // sync on object shared by all siblings
+                if (ab.length == 0) {
+                    break; // we've recursed or the deserialization thread just completed
+                } else if (ab == m_abChildren) {
+                    byte[] empty = new byte[0];
+
+                    synchronized (empty) { // other threads may sync and wait on this object from sync(ab) above
+                        // first mark all siblings as in active serialization; this allows other threads to still sync
+                        // and wait while ensuring that this thread doesn't endless recurse
+                        for (Iterator<Component> siblings = siblings(); siblings.hasNext(); ) {
+                            siblings.next().m_abChildren = empty;
                         }
 
-                    // now read in the children
-                    DataInput in = new DataInputStream(new ByteArrayInputStream(ab));
-                    try
-                        {
-                        disassembleChildren(in, true);
-                        }
-                    catch (IOException e)
-                        {
-                        throw new IllegalStateException("IOException occurred in " + getIdentityConstant()
-                                + " during deferred read of child components", e);
+                        // now read in the children
+                        DataInput in = new DataInputStream(new ByteArrayInputStream(ab));
+                        try {
+                            disassembleChildren(in, false);
+                        } catch (IOException e) {
+                            throw new IllegalStateException("IOException occurred in " + getIdentityConstant()
+                                    + " during deferred read of child components", e);
+                        } finally {
+                            // finally make sure neither this nor any sibling retains hold of it (since it indicates that
+                            // deserialization is deferred)
+                            for (Iterator<Component> siblings = siblings(); siblings.hasNext(); ) {
+                                siblings.next().m_abChildren = null;
+                            }
                         }
                     }
                 }
             }
         }
+    }
 
     /**
      * Visitor pattern for children of this component, optionally including all siblings, and

javatools/src/main/java/org/xvm/asm/ConstantPool.java

@@ -2636,7 +2636,7 @@ protected void disassemble(DataInput in)
         {
         m_listConst.clear();
         m_mapConstants.clear();
-        m_mapLocators.clear();
+            m_mapLocators = new EnumMap<>(Format.class);
 
         // read the number of constants in the pool
         int cConst = readMagnitude(in);
@@ -3134,19 +3134,21 @@ private Map<Constant, Constant> ensureConstantLookup(Format format)
      *
      * @return the map from locator to Constant
      */
-     private Map<Object, Constant> ensureLocatorLookup(Format format)
-        {
+    private Map<Object, Constant> ensureLocatorLookup(Format format) {
         Map<Object, Constant> map = m_mapLocators.get(format);
-        if (map == null)
-            {
-            // m_mapLocators is an EnumMap, which is not thread-safe
-            synchronized (this)
-                {
-                map = m_mapLocators.computeIfAbsent(format, _format -> new ConcurrentHashMap<>());
+        if (map == null) {
+            // m_mapLocators is an EnumMap, which is not thread-safe; use copy-on-write
+            synchronized (this) {
+                map = m_mapLocators.get(format);
+                if (map == null) {
+                    EnumMap<Format, Map<Object, Constant>> mapNew = new EnumMap<>(m_mapLocators);
+                    mapNew.put(format, map = new ConcurrentHashMap<>());
+                    m_mapLocators = mapNew;
                 }
             }
-        return map;
         }
+        return map;
+    }
 
     /**
      * Create the necessary structures for looking up Constant objects quickly, and populate those
@@ -3843,10 +3845,12 @@ public static Auto withPool(ConstantPool pool) {
      */
     private final EnumMap<Format, Map<Constant, Constant>> m_mapConstants = new EnumMap<>(Format.class);
 
-    /**
-     * Reverse lookup structure to find a particular constant by locator.
-     */
-    private final EnumMap<Format, Map<Object, Constant>> m_mapLocators = new EnumMap<>(Format.class);
+        /**
+         * Reverse lookup structure to find a particular constant by locator.
+         * <p>
+         * This map is not thread-safe and safety is provided via copy-on-write
+         */
+        private volatile EnumMap<Format, Map<Object, Constant>> m_mapLocators = new EnumMap<>(Format.class);
 
     /**
      * Set of references to ConstantPool instances, defining the only ConstantPool references that
@@ -4171,9 +4175,9 @@ private void optimize()
             }
 
         // discard any previous lookup structures, since contents may have changed
-        m_mapConstants.clear();
-        m_mapLocators.clear();
-        f_implicits.clear();
+            m_mapConstants.clear();
+            m_mapLocators = new EnumMap<>(Format.class);
+            f_implicits.clear();
         }
     private transient TypeConstant      m_typeRange;
     private transient TypeConstant      m_typeInterval;