fix deadlocks and other misbehaviors in class initialization code

src/compile.cpp

@@ -6452,22 +6452,6 @@ class MyProcessor: public Processor {
     }
   }
 
-  virtual void
-  initClass(Thread* t, object c)
-  {
-    PROTECT(t, c);
-    
-    ACQUIRE(t, t->m->classLock);
-    if (classNeedsInit(t, c)) {
-      classVmFlags(t, c) |= InitFlag;
-      invoke(t, classInitializer(t, c), 0);
-      if (t->exception) {
-        t->exception = makeExceptionInInitializerError(t, t->exception);
-      }
-      classVmFlags(t, c) &= ~(NeedInitFlag | InitFlag);
-    }
-  }
-
   virtual void
   visitObjects(Thread* vmt, Heap::Visitor* v)
   {

src/interpret.cpp

@@ -26,6 +26,8 @@ const unsigned FrameMethodOffset = 2;
 const unsigned FrameIpOffset = 3;
 const unsigned FrameFootprint = 4;
 
+class ClassInitList;
+
 class Thread: public vm::Thread {
  public:
   static const unsigned StackSizeInBytes = 64 * 1024;
@@ -36,16 +38,39 @@ class Thread: public vm::Thread {
     ip(0),
     sp(0),
     frame(-1),
-    code(0)
+    code(0),
+    classInitList(0)
   { }
 
   unsigned ip;
   unsigned sp;
   int frame;
   object code;
+  ClassInitList* classInitList;
   uintptr_t stack[StackSizeInWords];
 };
 
+class ClassInitList {
+ public:
+  ClassInitList(Thread* t, object class_, ClassInitList* next):
+    t(t), class_(class_), next(next)
+  { }
+
+  static void push(Thread* t, object class_) {
+    t->classInitList = new (t->m->heap->allocate(sizeof(ClassInitList)))
+      ClassInitList(t, class_, t->classInitList);
+  }
+
+  void pop() {
+    t->classInitList = next;
+    t->m->heap->free(this, sizeof(ClassInitList));
+  }
+
+  Thread* t;
+  object class_;
+  ClassInitList* next;
+};
+
 inline void
 pushObject(Thread* t, object o)
 {
@@ -348,12 +373,14 @@ popFrame(Thread* t)
     }   
   }
   
-  if (UNLIKELY(methodVmFlags(t, method) & ClassInitFlag)) {
+  if (UNLIKELY(methodVmFlags(t, method) & ClassInitFlag)
-    if (t->exception) {
+      and t->classInitList)
-      t->exception = makeExceptionInInitializerError(t, t->exception);
+  {
-    }
+    assert(t, t->classInitList->class_ == methodClass(t, method));
-    classVmFlags(t, methodClass(t, method)) &= ~(NeedInitFlag | InitFlag);
+    
-    release(t, t->m->classLock);
+    t->classInitList->pop();
+
+    postInitClass(t, methodClass(t, method));
   }
 
   t->sp = frameBase(t, t->frame);
@@ -732,17 +759,21 @@ invokeNative(Thread* t, object method)
 bool
 classInit2(Thread* t, object class_, unsigned ipOffset)
 {
+  for (ClassInitList* list = t->classInitList; list; list = list->next) {
+    if (list->class_ == class_) {
+      return false;
+    }
+  }
+
   PROTECT(t, class_);
-  acquire(t, t->m->classLock);
+
-  if (classVmFlags(t, class_) & NeedInitFlag
+  if (preInitClass(t, class_)) {
-      and (classVmFlags(t, class_) & InitFlag) == 0)
+    ClassInitList::push(t, class_);
-  {
+    
-    classVmFlags(t, class_) |= InitFlag;
     t->code = classInitializer(t, class_);
     t->ip -= ipOffset;
     return true;
   } else {
-    release(t, t->m->classLock);
     return false;
   }
 }
@@ -3070,22 +3101,6 @@ class MyProcessor: public Processor {
     // ignore
   }
 
-  virtual void
-  initClass(vm::Thread* t, object c)
-  {
-    PROTECT(t, c);
-    
-    acquire(t, t->m->classLock);
-    if (classVmFlags(t, c) & NeedInitFlag
-        and (classVmFlags(t, c) & InitFlag) == 0)
-    {
-      classVmFlags(t, c) |= InitFlag;
-      invoke(t, classInitializer(t, c), 0);
-    } else {
-      release(t, t->m->classLock);
-    }
-  }
-
   virtual void
   visitObjects(vm::Thread* vmt, Heap::Visitor* v)
   {
@@ -3098,6 +3113,10 @@ class MyProcessor: public Processor {
         v->visit(reinterpret_cast<object*>(t->stack + (i * 2) + 1));
       }
     }
+
+    for (ClassInitList* list = t->classInitList; list; list = list->next) {
+      v->visit(reinterpret_cast<object*>(&(list->class_)));
+    }
   }
 
   virtual void

src/machine.cpp

@@ -1863,6 +1863,7 @@ Thread::Thread(Machine* m, object javaThread, Thread* parent):
   heapIndex(0),
   heapOffset(0),
   protector(0),
+  classInitStack(0),
   runnable(this),
   defaultHeap(static_cast<uintptr_t*>
               (m->heap->allocate(ThreadHeapSizeInBytes))),
@@ -2687,6 +2688,92 @@ resolveObjectArrayClass(Thread* t, object elementSpec)
   return resolveClass(t, spec);
 }
 
+bool
+classNeedsInit(Thread* t, object c)
+{
+  if (classVmFlags(t, c) & NeedInitFlag) {
+    if (classVmFlags(t, c) & InitFlag) {
+      // the class is currently being initialized.  If this the thread
+      // which is initializing it, we should not try to initialize it
+      // recursively.  Otherwise, we must wait for the responsible
+      // thread to finish.
+      for (Thread::ClassInitStack* s = t->classInitStack; s; s = s->next) {
+        if (s->class_ == c) {
+          return false;
+        }
+      }
+    }
+    return true;
+  } else {
+    return false;
+  }
+}
+
+bool
+preInitClass(Thread* t, object c)
+{
+  if (classVmFlags(t, c) & NeedInitFlag) {
+    PROTECT(t, c);
+    ACQUIRE(t, t->m->classLock);
+    if (classVmFlags(t, c) & NeedInitFlag) {
+      if (classVmFlags(t, c) & InitFlag) {
+        // the class is currently being initialized.  If this the
+        // thread which is initializing it, we should not try to
+        // initialize it recursively.
+        for (Thread::ClassInitStack* s = t->classInitStack; s; s = s->next) {
+          if (s->class_ == c) {
+            return false;
+          }
+        }
+
+        // some other thread is on the job - wait for it to finish.
+        while (classVmFlags(t, c) & InitFlag) {
+          ENTER(t, Thread::IdleState);
+          t->m->classLock->wait(t->systemThread, 0);
+        }
+      } else if (classVmFlags(t, c) & InitErrorFlag) {
+        object message = makeString
+          (t, "%s", &byteArrayBody(t, className(t, c), 0));
+        t->exception = makeNoClassDefFoundError(t, message);
+      } else {
+        classVmFlags(t, c) |= InitFlag;
+        return true;
+      }
+    }
+  }
+  return false;
+}
+
+void
+postInitClass(Thread* t, object c)
+{
+  PROTECT(t, c);
+
+  ACQUIRE(t, t->m->classLock);
+  if (t->exception) {
+    t->exception = makeExceptionInInitializerError(t, t->exception);
+    classVmFlags(t, c) |= NeedInitFlag | InitErrorFlag;
+    classVmFlags(t, c) &= ~InitFlag;
+  } else {
+    classVmFlags(t, c) &= ~(NeedInitFlag | InitFlag);
+  }
+  t->m->classLock->notifyAll(t->systemThread);
+}
+
+void
+initClass(Thread* t, object c)
+{
+  PROTECT(t, c);
+
+  if (preInitClass(t, c)) {
+    Thread::ClassInitStack stack(t, c);
+
+    t->m->processor->invoke(t, classInitializer(t, c), 0);
+
+    postInitClass(t, c);
+  }
+}
+
 object
 makeObjectArray(Thread* t, object elementClass, unsigned count)
 {

src/machine.h

@@ -84,9 +84,10 @@ const unsigned ReferenceFlag = 1 << 0;
 const unsigned WeakReferenceFlag = 1 << 1;
 const unsigned NeedInitFlag = 1 << 2;
 const unsigned InitFlag = 1 << 3;
-const unsigned PrimitiveFlag = 1 << 4;
+const unsigned InitErrorFlag = 1 << 4;
-const unsigned BootstrapFlag = 1 << 5;
+const unsigned PrimitiveFlag = 1 << 5;
-const unsigned HasFinalMemberFlag = 1 << 6;
+const unsigned BootstrapFlag = 1 << 6;
+const unsigned HasFinalMemberFlag = 1 << 7;
 
 // method vmFlags:
 const unsigned ClassInitFlag = 1 << 0;
@@ -1267,6 +1268,25 @@ class Thread {
     object* p;
   };
 
+  class ClassInitStack {
+   public:
+    ClassInitStack(Thread* t, object class_):
+      next(t->classInitStack),
+      class_(class_),
+      protector(t, &(this->class_))
+    {
+      t->classInitStack = this;
+    }
+
+    ~ClassInitStack() {
+      protector.t->classInitStack = next;
+    }
+
+    ClassInitStack* next;
+    object class_;
+    SingleProtector protector;
+  };
+
   class Runnable: public System::Runnable {
    public:
     Runnable(Thread* t): t(t) { }
@@ -1319,6 +1339,7 @@ class Thread {
   unsigned heapIndex;
   unsigned heapOffset;
   Protector* protector;
+  ClassInitStack* classInitStack;
   Runnable runnable;
   uintptr_t* defaultHeap;
   uintptr_t* heap;
@@ -1763,6 +1784,14 @@ makeNoSuchMethodError(Thread* t, object message)
   return makeNoSuchMethodError(t, message, trace, 0);
 }
 
+inline object
+makeNoClassDefFoundError(Thread* t, object message)
+{
+  PROTECT(t, message);
+  object trace = makeTrace(t);
+  return makeNoClassDefFoundError(t, message, trace, 0);
+}
+
 inline object
 makeUnsatisfiedLinkError(Thread* t, object message)
 {
@@ -2100,20 +2129,17 @@ resolveField(Thread* t, const char* className, const char* fieldName,
 object
 resolveObjectArrayClass(Thread* t, object elementSpec);
 
-inline bool
+bool
-classNeedsInit(Thread* t, object c)
+classNeedsInit(Thread* t, object c);
-{
-  return classVmFlags(t, c) & NeedInitFlag
-    and (classVmFlags(t, c) & InitFlag) == 0;
-}
 
-inline void
+bool
-initClass(Thread* t, object c)
+preInitClass(Thread* t, object c);
-{
+
-  if (classNeedsInit(t, c)) {
+void
-    t->m->processor->initClass(t, c);
+postInitClass(Thread* t, object c);
-  }
+
-}
+void
+initClass(Thread* t, object c);
 
 object
 makeObjectArray(Thread* t, object elementClass, unsigned count);

src/processor.h

@@ -78,9 +78,6 @@ class Processor {
   virtual void
   initVtable(Thread* t, object c) = 0;
 
-  virtual void
-  initClass(Thread* t, object c) = 0;
-
   virtual void
   visitObjects(Thread* t, Heap::Visitor* v) = 0;
 

src/types.def

@@ -174,6 +174,8 @@
 
 (type noSuchMethodError java/lang/NoSuchMethodError)
 
+(type noClassDefFoundError java/lang/NoClassDefFoundError)
+
 (type unsatisfiedLinkError java/lang/UnsatisfiedLinkError)
 
 (type exceptionInInitializerError java/lang/ExceptionInInitializerError)