third attempt to properly fix race condition in compile function

src/compile.cpp

@@ -3610,21 +3610,6 @@ compareTraceElementPointers(const void* va, const void* vb)
   }
 }
 
-intptr_t
-compareMethodBounds(Thread* t, object a, object b)
-{
-  if (DebugMethodTree) {
-    fprintf(stderr, "compare %p to %p\n",
-            &singletonValue(t, methodCompiled(t, a), 0),
-            &singletonValue(t, methodCompiled(t, b), 0));
-  }
-
-  return reinterpret_cast<intptr_t>
-    (&singletonValue(t, methodCompiled(t, a), 0))
-    -  reinterpret_cast<intptr_t>
-    (&singletonValue(t, methodCompiled(t, b), 0));
-}
-
 unsigned
 frameObjectMapSize(MyThread* t, object method, object map)
 {
@@ -5074,40 +5059,78 @@ compile(MyThread* t, object method)
 {
   MyProcessor* p = processor(t);
 
-  PROTECT(t, method);
-
-  ACQUIRE(t, t->m->classLock);
-    
   if (methodCompiled(t, method) == p->defaultThunk) {
-    initClass(t, methodClass(t, method));
+    PROTECT(t, method);
-    if (UNLIKELY(t->exception)) return;
 
+    ACQUIRE(t, t->m->classLock);
+    
     if (methodCompiled(t, method) == p->defaultThunk) {
-      object compiled;
+      initClass(t, methodClass(t, method));
-      if (methodFlags(t, method) & ACC_NATIVE) {
+      if (UNLIKELY(t->exception)) return;
-        compiled = p->nativeThunk;
-      } else {
-        Context context(t, method);
-        compiled = compile(t, &context);
-        if (UNLIKELY(t->exception)) return;
-      }
 
-      set(t, method, MethodCompiled, compiled);
+      if (methodCompiled(t, method) == p->defaultThunk) {
-
+        object node;
-      if ((methodFlags(t, method) & ACC_NATIVE) == 0) {
+        object compiled;
-        if (DebugMethodTree) {
+        if (methodFlags(t, method) & ACC_NATIVE) {
-          fprintf(stderr, "insert method at %p\n",
+          node = 0;
-                  &singletonValue(t, methodCompiled(t, method), 0));
+          compiled = p->nativeThunk;
-        }
+        } else {
+          Context context(t, method);
+          compiled = compile(t, &context);
+          if (UNLIKELY(t->exception)) return;
           
-        methodTree(t) = treeInsert
+          PROTECT(t, compiled);
-          (t, methodTree(t), method, methodTreeSentinal(t),
-           compareMethodBounds);
-      }
 
-      if (methodVirtual(t, method)) {
+          if (DebugMethodTree) {
-        classVtable(t, methodClass(t, method), methodOffset(t, method))
+            fprintf(stderr, "insert method at %p\n",
-          = &singletonValue(t, methodCompiled(t, method), 0);
+                    &singletonValue(t, compiled, 0));
+          }
+
+          // We can't set the MethodCompiled field on the original
+          // method before it is placed into the method tree, since
+          // another thread might call the method, from which stack
+          // unwinding would fail (since there is not yet an entry in
+          // the method tree).  However, we can't insert the original
+          // method into the tree before setting the MethodCompiled
+          // field on it since we rely on that field to determine its
+          // position in the tree.  Therefore, we insert a clone in
+          // its place.  Later, we'll replace the clone with the
+          // original to save memory.
+
+          object clone = makeMethod
+            (t, methodVmFlags(t, method),
+             methodReturnCode(t, method),
+             methodParameterCount(t, method),
+             methodParameterFootprint(t, method),
+             methodFlags(t, method),
+             methodOffset(t, method),
+             methodName(t, method),
+             methodSpec(t, method),
+             methodClass(t, method),
+             methodCode(t, method),
+             compiled);
+
+          node = makeTreeNode
+            (t, clone, methodTreeSentinal(t), methodTreeSentinal(t));
+
+          PROTECT(t, node);
+
+          methodTree(t) = treeInsertNode
+            (t, methodTree(t), reinterpret_cast<intptr_t>
+             (&singletonValue(t, compiled, 0)), node, methodTreeSentinal(t),
+             compareIpToMethodBounds);
+        }
+
+        set(t, method, MethodCompiled, compiled);
+
+        if (methodVirtual(t, method)) {
+          classVtable(t, methodClass(t, method), methodOffset(t, method))
+            = &singletonValue(t, compiled, 0);
+        }
+
+        if (node) {
+          set(t, node, TreeNodeValue, method);
+        }
       }
     }
   }

src/util.cpp

@@ -59,8 +59,8 @@ cloneTreeNode(Thread* t, object n)
 }
 
 object
-treeFind(Thread* t, object old, object node, object sentinal,
+treeFind(Thread* t, object old, intptr_t key, object node, object sentinal,
-         intptr_t (*compare)(Thread* t, object a, object b))
+         intptr_t (*compare)(Thread* t, intptr_t key, object b))
 {
   PROTECT(t, old);
   PROTECT(t, node);
@@ -78,8 +78,7 @@ treeFind(Thread* t, object old, object node, object sentinal,
   while (old != sentinal) {
     ancestors = makePair(t, new_, ancestors);
 
-    intptr_t difference = compare
+    intptr_t difference = compare(t, key, getTreeNodeValue(t, old));
-      (t, getTreeNodeValue(t, node), getTreeNodeValue(t, old));
 
     if (difference < 0) {
       old = treeNodeLeft(t, old);
@@ -536,20 +535,16 @@ treeQuery(Thread* t, object tree, intptr_t key, object sentinal,
 }
 
 object
-treeInsert(Thread* t, object tree, object value, object sentinal,
+treeInsertNode(Thread* t, object tree, intptr_t key, object node,
-           intptr_t (*compare)(Thread* t, object a, object b))
+               object sentinal,
+               intptr_t (*compare)(Thread* t, intptr_t key, object b))
 {
   PROTECT(t, tree);
   PROTECT(t, sentinal);
 
-  object node = makeTreeNode(t, value, sentinal, sentinal);
+  object path = treeFind(t, tree, key, node, sentinal, compare);
-          
+  expect(t, treePathFresh(t, path));
-  object path = treeFind(t, tree, node, sentinal, compare);
+  return treeAdd(t, path);
-  if (treePathFresh(t, path)) {
-    return treeAdd(t, path);
-  } else {
-    return tree;
-  }  
 }
 
 } // namespace vm

src/util.h

@@ -88,8 +88,9 @@ treeQuery(Thread* t, object tree, intptr_t key, object sentinal,
           intptr_t (*compare)(Thread* t, intptr_t key, object b));
 
 object
-treeInsert(Thread* t, object tree, object value, object sentinal,
+treeInsertNode(Thread* t, object tree, intptr_t key, object node,
-           intptr_t (*compare)(Thread* t, object a, object b));
+               object sentinal,
+               intptr_t (*compare)(Thread* t, intptr_t key, object b));
 
 } // vm