You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by hw...@apache.org on 2010/03/19 23:18:21 UTC

svn commit: r925460 - in /subversion/trunk/subversion/bindings/javahl/native: ConflictResolverCallback.cpp CopySources.cpp EnumMapper.cpp JNIUtil.cpp JNIUtil.h ListCallback.cpp ProgressListener.cpp Revision.cpp

Author: hwright
Date: Fri Mar 19 22:18:21 2010
New Revision: 925460

URL: http://svn.apache.org/viewvc?rev=925460&view=rev
Log:
JavaHL: More reference framing.

[ in subversion/bindings/javahl/ ]
* native/ConflictResolverCallback.cpp
  (resolve, javaResultToC): Add a reference frame.

* native/JNIUtil.cpp
  (throwNativeException): Same.

* native/CopySources.cpp
  (makeJCopySources): Same.

* native/Revision.cpp
  (Revision): Same.

* native/EnumMapper.cpp
  (mapEnum, getName): Same.

* native/ProgressListener.cpp
  (onProgress): Same.

* native/ListCallback.cpp
  (createJavaDirEntry): Same.

* native/JNIUtil.h
  (POP_AND_RETURN): Don't put parens around the return value, to enable
    use in void methods.

Modified:
    subversion/trunk/subversion/bindings/javahl/native/ConflictResolverCallback.cpp
    subversion/trunk/subversion/bindings/javahl/native/CopySources.cpp
    subversion/trunk/subversion/bindings/javahl/native/EnumMapper.cpp
    subversion/trunk/subversion/bindings/javahl/native/JNIUtil.cpp
    subversion/trunk/subversion/bindings/javahl/native/JNIUtil.h
    subversion/trunk/subversion/bindings/javahl/native/ListCallback.cpp
    subversion/trunk/subversion/bindings/javahl/native/ProgressListener.cpp
    subversion/trunk/subversion/bindings/javahl/native/Revision.cpp

Modified: subversion/trunk/subversion/bindings/javahl/native/ConflictResolverCallback.cpp
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/javahl/native/ConflictResolverCallback.cpp?rev=925460&r1=925459&r2=925460&view=diff
==============================================================================
--- subversion/trunk/subversion/bindings/javahl/native/ConflictResolverCallback.cpp (original)
+++ subversion/trunk/subversion/bindings/javahl/native/ConflictResolverCallback.cpp Fri Mar 19 22:18:21 2010
@@ -99,6 +99,11 @@ ConflictResolverCallback::resolve(svn_wc
 {
   JNIEnv *env = JNIUtil::getEnv();
 
+  // Create a local frame for our references
+  env->PushLocalFrame(LOCAL_FRAME_SIZE);
+  if (JNIUtil::isJavaExceptionThrown())
+    return SVN_NO_ERROR;
+
   // As Java method IDs will not change during the time this library
   // is loaded, they can be cached.
   static jmethodID mid = 0;
@@ -107,23 +112,19 @@ ConflictResolverCallback::resolve(svn_wc
       // Initialize the callback method ID.
       jclass clazz = env->FindClass(JAVA_PACKAGE "/callback/ConflictResolverCallback");
       if (JNIUtil::isJavaExceptionThrown())
-        return SVN_NO_ERROR;
+        POP_AND_RETURN(SVN_NO_ERROR);
 
       mid = env->GetMethodID(clazz, "resolve",
                              "(L" JAVA_PACKAGE "/ConflictDescriptor;)"
                              "L" JAVA_PACKAGE "/ConflictResult;");
       if (JNIUtil::isJavaExceptionThrown() || mid == 0)
-        return SVN_NO_ERROR;
-
-      env->DeleteLocalRef(clazz);
-      if (JNIUtil::isJavaExceptionThrown())
-        return SVN_NO_ERROR;
+        POP_AND_RETURN(SVN_NO_ERROR);
     }
 
   // Create an instance of the conflict descriptor.
   jobject jdesc = CreateJ::ConflictDescriptor(desc);
   if (JNIUtil::isJavaExceptionThrown())
-    return SVN_NO_ERROR;
+    POP_AND_RETURN(SVN_NO_ERROR);
 
   // Invoke the Java conflict resolver callback method using the descriptor.
   jobject jresult = env->CallObjectMethod(m_conflictResolver, mid, jdesc);
@@ -132,17 +133,20 @@ ConflictResolverCallback::resolve(svn_wc
       // If an exception is thrown by our conflict resolver, remove it
       // from the JNI env, and convert it into a Subversion error.
       const char *msg = JNIUtil::thrownExceptionToCString();
-      return svn_error_create(SVN_ERR_WC_CONFLICT_RESOLVER_FAILURE, NULL, msg);
+      svn_error_t *err = svn_error_create(SVN_ERR_WC_CONFLICT_RESOLVER_FAILURE,
+                                          NULL, msg);
+      env->PopLocalFrame(NULL);
+      return err;
     }
   *result = javaResultToC(jresult, pool);
   if (*result == NULL)
-    // Unable to convert the result into a C representation.
-    return svn_error_create(SVN_ERR_WC_CONFLICT_RESOLVER_FAILURE, NULL, NULL);
-
-  env->DeleteLocalRef(jdesc);
-  if (JNIUtil::isJavaExceptionThrown())
-    return SVN_NO_ERROR;
+    {
+      // Unable to convert the result into a C representation.
+      env->PopLocalFrame(NULL);
+      return svn_error_create(SVN_ERR_WC_CONFLICT_RESOLVER_FAILURE, NULL, NULL);
+    }
 
+  env->PopLocalFrame(NULL);
   return SVN_NO_ERROR;
 }
 
@@ -150,6 +154,12 @@ svn_wc_conflict_result_t *
 ConflictResolverCallback::javaResultToC(jobject jresult, apr_pool_t *pool)
 {
   JNIEnv *env = JNIUtil::getEnv();
+
+  // Create a local frame for our references
+  env->PushLocalFrame(LOCAL_FRAME_SIZE);
+  if (JNIUtil::isJavaExceptionThrown())
+    return SVN_NO_ERROR;
+
   static jmethodID getChoice = 0;
   static jmethodID getMergedPath = 0;
 
@@ -158,7 +168,7 @@ ConflictResolverCallback::javaResultToC(
     {
       clazz = env->FindClass(JAVA_PACKAGE "/ConflictResult");
       if (JNIUtil::isJavaExceptionThrown())
-        return NULL;
+        POP_AND_RETURN_NULL;
     }
 
   if (getChoice == 0)
@@ -166,41 +176,33 @@ ConflictResolverCallback::javaResultToC(
       getChoice = env->GetMethodID(clazz, "getChoice",
                                    "()L"JAVA_PACKAGE"/ConflictResult$Choice;");
       if (JNIUtil::isJavaExceptionThrown() || getChoice == 0)
-        return NULL;
+        POP_AND_RETURN_NULL;
     }
   if (getMergedPath == 0)
     {
       getMergedPath = env->GetMethodID(clazz, "getMergedPath",
                                        "()Ljava/lang/String;");
       if (JNIUtil::isJavaExceptionThrown() || getMergedPath == 0)
-        return NULL;
-    }
-
-  if (clazz)
-    {
-      env->DeleteLocalRef(clazz);
-      if (JNIUtil::isJavaExceptionThrown())
-        return NULL;
+        POP_AND_RETURN_NULL;
     }
 
   jobject jchoice = env->CallObjectMethod(jresult, getChoice);
   if (JNIUtil::isJavaExceptionThrown())
-    return NULL;
+    POP_AND_RETURN_NULL;
 
   jstring jmergedPath = (jstring) env->CallObjectMethod(jresult, getMergedPath);
   if (JNIUtil::isJavaExceptionThrown())
-    return NULL;
+    POP_AND_RETURN_NULL;
 
   JNIStringHolder mergedPath(jmergedPath);
+  if (JNIUtil::isJavaExceptionThrown())
+    POP_AND_RETURN_NULL;
 
   svn_wc_conflict_result_t *result =
          svn_wc_create_conflict_result(EnumMapper::toConflictChoice(jchoice),
                                        mergedPath.pstrdup(pool),
                                        pool);
 
-  env->DeleteLocalRef(jchoice);
-  if (JNIUtil::isJavaExceptionThrown())
-    return NULL;
-
+  env->PopLocalFrame(NULL);
   return result;
 }

Modified: subversion/trunk/subversion/bindings/javahl/native/CopySources.cpp
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/javahl/native/CopySources.cpp?rev=925460&r1=925459&r2=925460&view=diff
==============================================================================
--- subversion/trunk/subversion/bindings/javahl/native/CopySources.cpp (original)
+++ subversion/trunk/subversion/bindings/javahl/native/CopySources.cpp Fri Mar 19 22:18:21 2010
@@ -47,17 +47,22 @@ CopySources::makeJCopySource(const char 
 {
   JNIEnv *env = JNIUtil::getEnv();
 
-  jobject jpath = JNIUtil::makeJString(path);
+  // Create a local frame for our references
+  env->PushLocalFrame(LOCAL_FRAME_SIZE);
   if (JNIUtil::isJavaExceptionThrown())
     return NULL;
 
+  jobject jpath = JNIUtil::makeJString(path);
+  if (JNIUtil::isJavaExceptionThrown())
+    POP_AND_RETURN_NULL;
+
   jobject jrevision = Revision::makeJRevision(rev);
   if (JNIUtil::isJavaExceptionThrown())
-    return NULL;
+    POP_AND_RETURN_NULL;
 
   jclass clazz = env->FindClass(JAVA_PACKAGE "/CopySource");
   if (JNIUtil::isJavaExceptionThrown())
-    return NULL;
+    POP_AND_RETURN_NULL;
 
   static jmethodID ctor = 0;
   if (ctor == 0)
@@ -67,22 +72,14 @@ CopySources::makeJCopySource(const char 
                               "L" JAVA_PACKAGE "/Revision;"
                               "L" JAVA_PACKAGE "/Revision;)V");
       if (JNIUtil::isExceptionThrown())
-        return NULL;
+        POP_AND_RETURN_NULL;
     }
 
   jobject jcopySource = env->NewObject(clazz, ctor, jpath, jrevision, NULL);
   if (JNIUtil::isJavaExceptionThrown())
-    return NULL;
-
-  env->DeleteLocalRef(jpath);
-  if (JNIUtil::isJavaExceptionThrown())
-    return NULL;
-
-  env->DeleteLocalRef(jrevision);
-  if (JNIUtil::isJavaExceptionThrown())
-    return NULL;
+    POP_AND_RETURN_NULL;
 
-  return jcopySource;
+  return env->PopLocalFrame(jcopySource);
 }
 
 apr_array_header_t *

Modified: subversion/trunk/subversion/bindings/javahl/native/EnumMapper.cpp
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/javahl/native/EnumMapper.cpp?rev=925460&r1=925459&r2=925460&view=diff
==============================================================================
--- subversion/trunk/subversion/bindings/javahl/native/EnumMapper.cpp (original)
+++ subversion/trunk/subversion/bindings/javahl/native/EnumMapper.cpp Fri Mar 19 22:18:21 2010
@@ -576,49 +576,51 @@ jobject EnumMapper::mapEnum(const char *
   methodSig.append(";");
 
   JNIEnv *env = JNIUtil::getEnv();
-  jclass clazz = env->FindClass(clazzName);
-  if (JNIUtil::isJavaExceptionThrown())
-    return NULL;
 
-  jmethodID mid = env->GetStaticMethodID(clazz, "valueOf", methodSig.c_str());
+  // Create a local frame for our references
+  env->PushLocalFrame(LOCAL_FRAME_SIZE);
   if (JNIUtil::isJavaExceptionThrown())
     return NULL;
 
-  jstring jname = JNIUtil::makeJString(name);
+  jclass clazz = env->FindClass(clazzName);
   if (JNIUtil::isJavaExceptionThrown())
-    return NULL;
+    POP_AND_RETURN_NULL;
 
-  jobject jdepth = env->CallStaticObjectMethod(clazz, mid, jname);
+  jmethodID mid = env->GetStaticMethodID(clazz, "valueOf", methodSig.c_str());
   if (JNIUtil::isJavaExceptionThrown())
-    return NULL;
+    POP_AND_RETURN_NULL;
 
-  env->DeleteLocalRef(jname);
+  jstring jname = JNIUtil::makeJString(name);
   if (JNIUtil::isJavaExceptionThrown())
-    return NULL;
+    POP_AND_RETURN_NULL;
 
-  env->DeleteLocalRef(clazz);
+  jobject jdepth = env->CallStaticObjectMethod(clazz, mid, jname);
   if (JNIUtil::isJavaExceptionThrown())
-    return NULL;
+    POP_AND_RETURN_NULL;
 
-  return jdepth;
+  return env->PopLocalFrame(jdepth);
 }
 
 jstring EnumMapper::getName(const char *clazzName, jobject jenum)
 {
   JNIEnv *env = JNIUtil::getEnv();
-  jclass clazz = env->FindClass(clazzName);
+
+  // Create a local frame for our references
+  env->PushLocalFrame(LOCAL_FRAME_SIZE);
   if (JNIUtil::isJavaExceptionThrown())
     return NULL;
 
+  jclass clazz = env->FindClass(clazzName);
+  if (JNIUtil::isJavaExceptionThrown())
+    POP_AND_RETURN_NULL;
+
   jmethodID mid = env->GetMethodID(clazz, "name", "()Ljava/lang/String;");
   if (JNIUtil::isJavaExceptionThrown())
-    return NULL;
+    POP_AND_RETURN_NULL;
 
   jstring jname = (jstring) env->CallObjectMethod(jenum, mid);
-
-  env->DeleteLocalRef(clazz);
   if (JNIUtil::isJavaExceptionThrown())
-    return NULL;
+    POP_AND_RETURN_NULL;
 
-  return jname;
+  return (jstring) env->PopLocalFrame(jname);
 }

Modified: subversion/trunk/subversion/bindings/javahl/native/JNIUtil.cpp
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/javahl/native/JNIUtil.cpp?rev=925460&r1=925459&r2=925460&view=diff
==============================================================================
--- subversion/trunk/subversion/bindings/javahl/native/JNIUtil.cpp (original)
+++ subversion/trunk/subversion/bindings/javahl/native/JNIUtil.cpp Fri Mar 19 22:18:21 2010
@@ -338,6 +338,11 @@ JNIUtil::throwNativeException(const char
   JNIEnv *env = getEnv();
   jclass clazz = env->FindClass(className);
 
+  // Create a local frame for our references
+  env->PushLocalFrame(LOCAL_FRAME_SIZE);
+  if (JNIUtil::isJavaExceptionThrown())
+    return;
+
   if (getLogLevel() >= exceptionLog)
     {
       JNICriticalSection cs(*g_logMutex);
@@ -350,37 +355,25 @@ JNIUtil::throwNativeException(const char
       g_logStream << std::endl;
     }
   if (isJavaExceptionThrown())
-    return;
+    POP_AND_RETURN();
 
   jstring jmessage = makeJString(msg);
   if (isJavaExceptionThrown())
-    return;
+    POP_AND_RETURN();
   jstring jsource = makeJString(source);
   if (isJavaExceptionThrown())
-    return;
+    POP_AND_RETURN();
 
   jmethodID mid = env->GetMethodID(clazz, "<init>",
                                    "(Ljava/lang/String;Ljava/lang/String;I)V");
   if (isJavaExceptionThrown())
-    return;
+    POP_AND_RETURN();
   jobject nativeException = env->NewObject(clazz, mid, jmessage, jsource,
                                            static_cast<jint>(aprErr));
   if (isJavaExceptionThrown())
-    return;
-
-  env->DeleteLocalRef(clazz);
-  if (isJavaExceptionThrown())
-    return;
-
-  env->DeleteLocalRef(jmessage);
-  if (isJavaExceptionThrown())
-    return;
-
-  env->DeleteLocalRef(jsource);
-  if (isJavaExceptionThrown())
-    return;
+    POP_AND_RETURN();
 
-  env->Throw(static_cast<jthrowable>(nativeException));
+  env->Throw(static_cast<jthrowable>(env->PopLocalFrame(nativeException)));
 }
 
 void JNIUtil::handleSVNError(svn_error_t *err)

Modified: subversion/trunk/subversion/bindings/javahl/native/JNIUtil.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/javahl/native/JNIUtil.h?rev=925460&r1=925459&r2=925460&view=diff
==============================================================================
--- subversion/trunk/subversion/bindings/javahl/native/JNIUtil.h (original)
+++ subversion/trunk/subversion/bindings/javahl/native/JNIUtil.h Fri Mar 19 22:18:21 2010
@@ -256,7 +256,7 @@ class JNIUtil
   do                                    \
     {                                   \
       env->PopLocalFrame(NULL);         \
-      return (ret_val);                 \
+      return ret_val ;                  \
     }                                   \
   while (0)
 

Modified: subversion/trunk/subversion/bindings/javahl/native/ListCallback.cpp
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/javahl/native/ListCallback.cpp?rev=925460&r1=925459&r2=925460&view=diff
==============================================================================
--- subversion/trunk/subversion/bindings/javahl/native/ListCallback.cpp (original)
+++ subversion/trunk/subversion/bindings/javahl/native/ListCallback.cpp Fri Mar 19 22:18:21 2010
@@ -125,9 +125,15 @@ ListCallback::createJavaDirEntry(const c
                                  const svn_dirent_t *dirent)
 {
   JNIEnv *env = JNIUtil::getEnv();
+
+  // Create a local frame for our references
+  env->PushLocalFrame(LOCAL_FRAME_SIZE);
+  if (JNIUtil::isJavaExceptionThrown())
+    return SVN_NO_ERROR;
+
   jclass clazz = env->FindClass(JAVA_PACKAGE"/DirEntry");
   if (JNIUtil::isJavaExceptionThrown())
-    return NULL;
+    POP_AND_RETURN_NULL;
 
   static jmethodID mid = 0;
   if (mid == 0)
@@ -137,20 +143,20 @@ ListCallback::createJavaDirEntry(const c
                              "L"JAVA_PACKAGE"/NodeKind;"
                              "JZJJLjava/lang/String;)V");
       if (JNIUtil::isJavaExceptionThrown())
-        return NULL;
+        POP_AND_RETURN_NULL;
     }
 
   jstring jPath = JNIUtil::makeJString(path);
   if (JNIUtil::isJavaExceptionThrown())
-    return NULL;
+    POP_AND_RETURN_NULL;
 
   jstring jAbsPath = JNIUtil::makeJString(absPath);
   if (JNIUtil::isJavaExceptionThrown())
-    return NULL;
+    POP_AND_RETURN_NULL;
 
   jobject jNodeKind = EnumMapper::mapNodeKind(dirent->kind);
   if (JNIUtil::isJavaExceptionThrown())
-    return NULL;
+    POP_AND_RETURN_NULL;
 
   jlong jSize = dirent->size;
   jboolean jHasProps = (dirent->has_props? JNI_TRUE : JNI_FALSE);
@@ -158,32 +164,13 @@ ListCallback::createJavaDirEntry(const c
   jlong jLastChanged = dirent->time;
   jstring jLastAuthor = JNIUtil::makeJString(dirent->last_author);
   if (JNIUtil::isJavaExceptionThrown())
-    return NULL;
+    POP_AND_RETURN_NULL;
 
   jobject ret = env->NewObject(clazz, mid, jPath, jAbsPath, jNodeKind,
                                jSize, jHasProps, jLastChangedRevision,
                                jLastChanged, jLastAuthor);
   if (JNIUtil::isJavaExceptionThrown())
-    return NULL;
-
-  env->DeleteLocalRef(clazz);
-  if (JNIUtil::isJavaExceptionThrown())
-    return NULL;
-
-  env->DeleteLocalRef(jNodeKind);
-  if (JNIUtil::isJavaExceptionThrown())
-    return NULL;
-
-  env->DeleteLocalRef(jPath);
-  if (JNIUtil::isJavaExceptionThrown())
-    return NULL;
-
-  if (jLastAuthor != NULL)
-    {
-      env->DeleteLocalRef(jLastAuthor);
-      if (JNIUtil::isJavaExceptionThrown())
-        return NULL;
-    }
+    POP_AND_RETURN_NULL;
 
-  return ret;
+  return env->PopLocalFrame(ret);
 }

Modified: subversion/trunk/subversion/bindings/javahl/native/ProgressListener.cpp
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/javahl/native/ProgressListener.cpp?rev=925460&r1=925459&r2=925460&view=diff
==============================================================================
--- subversion/trunk/subversion/bindings/javahl/native/ProgressListener.cpp (original)
+++ subversion/trunk/subversion/bindings/javahl/native/ProgressListener.cpp Fri Mar 19 22:18:21 2010
@@ -87,6 +87,11 @@ ProgressListener::onProgress(apr_off_t p
 {
   JNIEnv *env = JNIUtil::getEnv();
 
+  // Create a local frame for our references
+  env->PushLocalFrame(LOCAL_FRAME_SIZE);
+  if (JNIUtil::isJavaExceptionThrown())
+    return;
+
   // As Java method IDs will not change during the time this library
   // is loaded, they can be cached.
   static jmethodID mid = 0;
@@ -95,45 +100,33 @@ ProgressListener::onProgress(apr_off_t p
       // Initialize the method ID.
       jclass clazz = env->FindClass(JAVA_PACKAGE"/ProgressListener");
       if (JNIUtil::isJavaExceptionThrown())
-        return;
+        POP_AND_RETURN();
 
       mid = env->GetMethodID(clazz, "onProgress",
                              "(L"JAVA_PACKAGE"/ProgressEvent;)V");
       if (JNIUtil::isJavaExceptionThrown() || mid == 0)
-        return;
-
-      env->DeleteLocalRef(clazz);
-      if (JNIUtil::isJavaExceptionThrown())
-        return;
+        POP_AND_RETURN();
     }
 
   static jmethodID midCT = 0;
   jclass clazz = env->FindClass(JAVA_PACKAGE"/ProgressEvent");
   if (JNIUtil::isJavaExceptionThrown())
-    return;
+    POP_AND_RETURN();
 
   if (midCT == 0)
     {
       midCT = env->GetMethodID(clazz, "<init>", "(JJ)V");
       if (JNIUtil::isJavaExceptionThrown() || midCT == 0)
-        return;
+        POP_AND_RETURN();
     }
 
   // Call the Java method.
   jobject jevent = env->NewObject(clazz, midCT,
                                   (jlong) progressVal, (jlong) total);
   if (JNIUtil::isJavaExceptionThrown())
-    return;
-
-  env->DeleteLocalRef(clazz);
-  if (JNIUtil::isJavaExceptionThrown())
-    return;
+    POP_AND_RETURN();
 
   env->CallVoidMethod(m_progressListener, mid, jevent);
-  if (JNIUtil::isJavaExceptionThrown())
-    return;
-
-  env->DeleteLocalRef(jevent);
-  if (JNIUtil::isJavaExceptionThrown())
-    return;
+  
+  POP_AND_RETURN();
 }

Modified: subversion/trunk/subversion/bindings/javahl/native/Revision.cpp
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/javahl/native/Revision.cpp?rev=925460&r1=925459&r2=925460&view=diff
==============================================================================
--- subversion/trunk/subversion/bindings/javahl/native/Revision.cpp (original)
+++ subversion/trunk/subversion/bindings/javahl/native/Revision.cpp Fri Mar 19 22:18:21 2010
@@ -48,25 +48,27 @@ Revision::Revision(jobject jthis, bool h
   else
     {
       JNIEnv *env = JNIUtil::getEnv();
+
+      // Create a local frame for our references
+      env->PushLocalFrame(LOCAL_FRAME_SIZE);
+      if (JNIUtil::isJavaExceptionThrown())
+        return;
+
       static jfieldID fid = 0;
       if (fid == 0)
         {
           jclass clazz = env->FindClass(JAVA_PACKAGE"/Revision");
           if (JNIUtil::isJavaExceptionThrown())
-            return;
+            POP_AND_RETURN();
 
           fid = env->GetFieldID(clazz, "revKind",
                                 "L"JAVA_PACKAGE"/Revision$Kind;");
           if (JNIUtil::isJavaExceptionThrown())
-            return;
-
-          env->DeleteLocalRef(clazz);
-          if (JNIUtil::isJavaExceptionThrown())
-            return;
+            POP_AND_RETURN();
         }
       jobject jKind = env->GetObjectField(jthis, fid);
       if (JNIUtil::isJavaExceptionThrown())
-        return;
+        POP_AND_RETURN();
 
       m_revision.value.number = 0;
       m_revision.kind = EnumMapper::toRevisionKind(jKind);
@@ -81,15 +83,11 @@ Revision::Revision(jobject jthis, bool h
                 jclass clazz =
                   env->FindClass(JAVA_PACKAGE"/Revision$Number");
                 if (JNIUtil::isJavaExceptionThrown())
-                  return;
+                  POP_AND_RETURN();
 
                 fidNum = env->GetFieldID(clazz, "revNumber", "J");
                 if (JNIUtil::isJavaExceptionThrown())
-                  return;
-
-                env->DeleteLocalRef(clazz);
-                if (JNIUtil::isJavaExceptionThrown())
-                  return;
+                  POP_AND_RETURN();
               }
             jlong jNumber = env->GetLongField(jthis, fidNum);
             m_revision.value.number = (svn_revnum_t) jNumber;
@@ -103,43 +101,31 @@ Revision::Revision(jobject jthis, bool h
                 jclass clazz =
                   env->FindClass(JAVA_PACKAGE"/Revision$DateSpec");
                 if (JNIUtil::isJavaExceptionThrown())
-                  return;
+                  POP_AND_RETURN();
 
                 fidDate = env->GetFieldID(clazz, "revDate",
                                           "Ljava/util/Date;");
                 if (JNIUtil::isJavaExceptionThrown())
-                  return;
-
-                env->DeleteLocalRef(clazz);
-                if (JNIUtil::isJavaExceptionThrown())
-                  return;
+                  POP_AND_RETURN();
               }
             jobject jDate = env->GetObjectField(jthis, fidDate);
             if (JNIUtil::isJavaExceptionThrown())
-              return;
+              POP_AND_RETURN();
 
             static jmethodID mid = 0;
             if (mid == 0)
               {
                 jclass clazz = env->FindClass("java/util/Date");
                 if (JNIUtil::isJavaExceptionThrown())
-                  return;
+                  POP_AND_RETURN();
 
                 mid = env->GetMethodID(clazz, "getTime", "()J");
                 if (JNIUtil::isJavaExceptionThrown())
-                  return;
-
-                env->DeleteLocalRef(clazz);
-                if (JNIUtil::isJavaExceptionThrown())
-                  return;
+                  POP_AND_RETURN();
               }
             jlong jMillSec = env->CallLongMethod(jDate, mid);
             if (JNIUtil::isJavaExceptionThrown())
-              return;
-
-            env->DeleteLocalRef(jDate);
-            if (JNIUtil::isJavaExceptionThrown())
-              return;
+              POP_AND_RETURN();
 
             m_revision.value.date = jMillSec * 1000;
           }
@@ -148,6 +134,7 @@ Revision::Revision(jobject jthis, bool h
           /* None of the other revision kinds need special handling. */
           break;
         }
+      env->PopLocalFrame(NULL);
     }
   if (headIfUnspecified && m_revision.kind == svn_opt_revision_unspecified)
     m_revision.kind = svn_opt_revision_head;