From b9f8188544aa44c5a2b8ab8e24165d37276bccb8 Mon Sep 17 00:00:00 2001
From: Joel Dice <joel.dice@gmail.com>
Date: Fri, 25 Mar 2011 18:37:02 -0600
Subject: [PATCH] don't try to release monitor if we get OOME when trying to
 acquire it

We can't blindly try release the monitors for all synchronized methods
when unwinding the stack since we may not have finished acquiring the
most recent one when the exception was thrown.
---
 src/compile.cpp   | 43 ++++++++++++++++++++++++++++++++-----------
 src/interpret.cpp | 30 ++++++++++++++++++------------
 src/thunks.cpp    |  1 +
 3 files changed, 51 insertions(+), 23 deletions(-)

diff --git a/src/compile.cpp b/src/compile.cpp
index 8e7f2ef1ee..2176f17d57 100644
--- a/src/compile.cpp
+++ b/src/compile.cpp
@@ -235,7 +235,8 @@ class MyThread: public Thread {
          : makeArchitecture(m->system, useNativeFeatures)),
     transition(0),
     traceContext(0),
-    stackLimit(0)
+    stackLimit(0),
+    methodLockIsClean(true)
   {
     arch->acquire();
   }
@@ -256,6 +257,7 @@ class MyThread: public Thread {
   Context* transition;
   TraceContext* traceContext;
   uintptr_t stackLimit;
+  bool methodLockIsClean;
 };
 
 void
@@ -1982,16 +1984,23 @@ void
 releaseLock(MyThread* t, object method, void* stack)
 {
   if (methodFlags(t, method) & ACC_SYNCHRONIZED) {
-    object lock;
-    if (methodFlags(t, method) & ACC_STATIC) {
-      lock = methodClass(t, method);
-    } else {
-      lock = *localObject
-        (t, stackForFrame(t, stack, method), method,
-         savedTargetIndex(t, method));
-    }
+    if (t->methodLockIsClean) {
+      object lock;
+      if (methodFlags(t, method) & ACC_STATIC) {
+        lock = methodClass(t, method);
+      } else {
+        lock = *localObject
+          (t, stackForFrame(t, stack, method), method,
+           savedTargetIndex(t, method));
+      }
     
-    release(t, lock);
+      release(t, lock);
+    } else {
+      // got an exception while trying to acquire the lock for a
+      // synchronized method -- don't try to release it, since we
+      // never succeeded in acquiring it.
+      t->methodLockIsClean = true;
+    }
   }
 }
 
@@ -2764,6 +2773,18 @@ acquireMonitorForObject(MyThread* t, object o)
   }
 }
 
+void
+acquireMonitorForObjectOnEntrance(MyThread* t, object o)
+{
+  if (LIKELY(o)) {
+    t->methodLockIsClean = false;
+    acquire(t, o);
+    t->methodLockIsClean = true;
+  } else {
+    throwNew(t, Machine::NullPointerExceptionType);
+  }
+}
+
 void
 releaseMonitorForObject(MyThread* t, object o)
 {
@@ -3456,7 +3477,7 @@ handleEntrance(MyThread* t, Frame* frame)
   }
 
   handleMonitorEvent
-    (t, frame, getThunk(t, acquireMonitorForObjectThunk));
+    (t, frame, getThunk(t, acquireMonitorForObjectOnEntranceThunk));
 }
 
 void
diff --git a/src/interpret.cpp b/src/interpret.cpp
index 482859e21f..13f3b9e853 100644
--- a/src/interpret.cpp
+++ b/src/interpret.cpp
@@ -322,15 +322,29 @@ setLocalLong(Thread* t, unsigned index, uint64_t value)
 void
 pushFrame(Thread* t, object method)
 {
-  if (t->frame >= 0) {
-    pokeInt(t, t->frame + FrameIpOffset, t->ip);
-  }
-  t->ip = 0;
+  PROTECT(t, method);
 
   unsigned parameterFootprint = methodParameterFootprint(t, method);
   unsigned base = t->sp - parameterFootprint;
   unsigned locals = parameterFootprint;
 
+  if (methodFlags(t, method) & ACC_SYNCHRONIZED) {
+    // Try to acquire the monitor before doing anything else.
+    // Otherwise, if we were to push the frame first, we risk trying
+    // to release a monitor we never successfully acquired when we try
+    // to pop the frame back off.
+    if (methodFlags(t, method) & ACC_STATIC) {
+      acquire(t, methodClass(t, method));
+    } else {
+      acquire(t, peekObject(t, base));
+    }   
+  }
+
+  if (t->frame >= 0) {
+    pokeInt(t, t->frame + FrameIpOffset, t->ip);
+  }
+  t->ip = 0;
+
   if ((methodFlags(t, method) & ACC_NATIVE) == 0) {
     t->code = methodCode(t, method);
 
@@ -349,14 +363,6 @@ pushFrame(Thread* t, object method)
   pokeInt(t, frame + FrameBaseOffset, base);
   pokeObject(t, frame + FrameMethodOffset, method);
   pokeInt(t, t->frame + FrameIpOffset, 0);
-
-  if (methodFlags(t, method) & ACC_SYNCHRONIZED) {
-    if (methodFlags(t, method) & ACC_STATIC) {
-      acquire(t, methodClass(t, method));
-    } else {
-      acquire(t, peekObject(t, base));
-    }   
-  }
 }
 
 void
diff --git a/src/thunks.cpp b/src/thunks.cpp
index dce0885c96..9279cbb811 100644
--- a/src/thunks.cpp
+++ b/src/thunks.cpp
@@ -45,6 +45,7 @@ THUNK(makeBlankArray)
 THUNK(lookUpAddress)
 THUNK(setMaybeNull)
 THUNK(acquireMonitorForObject)
+THUNK(acquireMonitorForObjectOnEntrance)
 THUNK(releaseMonitorForObject)
 THUNK(makeMultidimensionalArray)
 THUNK(makeMultidimensionalArrayFromReference)