subversion-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From hwri...@apache.org
Subject svn commit: r925387 - in /subversion/trunk/subversion/bindings/javahl/native: BlameCallback.cpp CreateJ.cpp DiffSummaryReceiver.cpp JNIUtil.h ListCallback.cpp LogMessageCallback.cpp ProplistCallback.cpp
Date Fri, 19 Mar 2010 19:22:42 GMT
Author: hwright
Date: Fri Mar 19 19:22:41 2010
New Revision: 925387

URL: http://svn.apache.org/viewvc?rev=925387&view=rev
Log:
JavaHL: Add some more uses of PushLocalFrame/PopLocalFrame for managing
local object references in JNI.

[ in subversion/bindings/javahl/ ]
* native/DiffSummaryReceiver.cpp
  (onSummary),
* native/ProplistCallback.cpp
  (singlePath),
* native/LogMessageCallback.cpp
  (single_message),
* native/ListCallback.cpp
  (doList),
* native/BlameCallback.cpp
  (singleLine):
    Apply the Push/Pop reference frame technique to ensure our local references
    aren't leaked.

* native/CreateJ.cpp
  (LOCAL_FRAME_SIZE, POP_AND_RETURN_NULL): Remove.

* native/JNIUtil.h
  (LOCAL_FRAME_SIZE, POP_AND_RETURN, POP_AND_RETURN_NULL): New.

Modified:
    subversion/trunk/subversion/bindings/javahl/native/BlameCallback.cpp
    subversion/trunk/subversion/bindings/javahl/native/CreateJ.cpp
    subversion/trunk/subversion/bindings/javahl/native/DiffSummaryReceiver.cpp
    subversion/trunk/subversion/bindings/javahl/native/JNIUtil.h
    subversion/trunk/subversion/bindings/javahl/native/ListCallback.cpp
    subversion/trunk/subversion/bindings/javahl/native/LogMessageCallback.cpp
    subversion/trunk/subversion/bindings/javahl/native/ProplistCallback.cpp

Modified: subversion/trunk/subversion/bindings/javahl/native/BlameCallback.cpp
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/javahl/native/BlameCallback.cpp?rev=925387&r1=925386&r2=925387&view=diff
==============================================================================
--- subversion/trunk/subversion/bindings/javahl/native/BlameCallback.cpp (original)
+++ subversion/trunk/subversion/bindings/javahl/native/BlameCallback.cpp Fri Mar 19 19:22:41
2010
@@ -79,6 +79,11 @@ BlameCallback::singleLine(apr_int64_t li
 {
   JNIEnv *env = JNIUtil::getEnv();
 
+  // Create a local frame for our references
+  env->PushLocalFrame(LOCAL_FRAME_SIZE);
+  if (JNIUtil::isJavaExceptionThrown())
+    return SVN_NO_ERROR;
+
   // The method id will not change during the time this library is
   // loaded, so it can be cached.
   static jmethodID mid = 0;
@@ -86,62 +91,42 @@ BlameCallback::singleLine(apr_int64_t li
     {
       jclass clazz = env->FindClass(JAVA_PACKAGE"/callback/BlameCallback");
       if (JNIUtil::isJavaExceptionThrown())
-        return SVN_NO_ERROR;
+        POP_AND_RETURN(SVN_NO_ERROR);
 
       mid = env->GetMethodID(clazz, "singleLine",
                              "(JJLjava/util/Map;JLjava/util/Map;"
                              "Ljava/lang/String;Ljava/lang/String;Z)V");
       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);
     }
 
   // convert the parameters to their Java relatives
   jobject jrevProps = CreateJ::PropertyMap(revProps, pool);
   if (JNIUtil::isJavaExceptionThrown())
-    return SVN_NO_ERROR;
+    POP_AND_RETURN(SVN_NO_ERROR);
 
   jobject jmergedRevProps = NULL;
   if (mergedRevProps != NULL)
     {
       jmergedRevProps = CreateJ::PropertyMap(mergedRevProps, pool);
       if (JNIUtil::isJavaExceptionThrown())
-        return SVN_NO_ERROR;
+        POP_AND_RETURN(SVN_NO_ERROR);
     }
 
   jstring jmergedPath = JNIUtil::makeJString(mergedPath);
   if (JNIUtil::isJavaExceptionThrown())
-    return SVN_NO_ERROR;
+    POP_AND_RETURN(SVN_NO_ERROR);
 
   jstring jline = JNIUtil::makeJString(line);
   if (JNIUtil::isJavaExceptionThrown())
-    return SVN_NO_ERROR;
+    POP_AND_RETURN(SVN_NO_ERROR);
 
   // call the Java method
   env->CallVoidMethod(m_callback, mid, (jlong)line_no, (jlong)revision,
                       jrevProps, (jlong)mergedRevision, jmergedRevProps,
                       jmergedPath, jline, (jboolean)localChange);
-  if (JNIUtil::isJavaExceptionThrown())
-    return SVN_NO_ERROR;
-
-  // cleanup the temporary Java objects
-  env->DeleteLocalRef(jrevProps);
-  if (JNIUtil::isJavaExceptionThrown())
-    return SVN_NO_ERROR;
-
-  env->DeleteLocalRef(jmergedRevProps);
-  if (JNIUtil::isJavaExceptionThrown())
-    return SVN_NO_ERROR;
-
-  env->DeleteLocalRef(jmergedPath);
-  if (JNIUtil::isJavaExceptionThrown())
-    return SVN_NO_ERROR;
-
-  env->DeleteLocalRef(jline);
   // No need to check for an exception here, because we return anyway.
 
+  env->PopLocalFrame(NULL);
   return SVN_NO_ERROR;
 }

Modified: subversion/trunk/subversion/bindings/javahl/native/CreateJ.cpp
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/javahl/native/CreateJ.cpp?rev=925387&r1=925386&r2=925387&view=diff
==============================================================================
--- subversion/trunk/subversion/bindings/javahl/native/CreateJ.cpp (original)
+++ subversion/trunk/subversion/bindings/javahl/native/CreateJ.cpp Fri Mar 19 19:22:41 2010
@@ -32,16 +32,6 @@
 #include "RevisionRange.h"
 #include "CreateJ.h"
 
-#define LOCAL_FRAME_SIZE            16
-
-#define POP_AND_RETURN_NULL         \
-  do                                \
-    {                               \
-      env->PopLocalFrame(NULL);     \
-      return NULL;                  \
-    }                               \
-  while (0)
-
 jobject
 CreateJ::ConflictDescriptor(const svn_wc_conflict_description_t *desc)
 {

Modified: subversion/trunk/subversion/bindings/javahl/native/DiffSummaryReceiver.cpp
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/javahl/native/DiffSummaryReceiver.cpp?rev=925387&r1=925386&r2=925387&view=diff
==============================================================================
--- subversion/trunk/subversion/bindings/javahl/native/DiffSummaryReceiver.cpp (original)
+++ subversion/trunk/subversion/bindings/javahl/native/DiffSummaryReceiver.cpp Fri Mar 19
19:22:41 2010
@@ -56,26 +56,27 @@ DiffSummaryReceiver::onSummary(const svn
                                apr_pool_t *pool)
 {
   JNIEnv *env = JNIUtil::getEnv();
-  jclass clazz;
+
+  // 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 callback = 0;
+  jclass clazz;
   if (callback == 0)
     {
       // Initialize the method ID.
       clazz = env->FindClass(JAVA_PACKAGE "/callback/DiffSummaryCallback");
       if (JNIUtil::isJavaExceptionThrown())
-        return SVN_NO_ERROR;
+        POP_AND_RETURN(SVN_NO_ERROR);
 
       callback = env->GetMethodID(clazz, "onSummary",
                                   "(L"JAVA_PACKAGE"/DiffSummary;)V");
       if (JNIUtil::isJavaExceptionThrown() || callback == 0)
-        return SVN_NO_ERROR;
-
-      env->DeleteLocalRef(clazz);
-      if (JNIUtil::isJavaExceptionThrown())
-        return SVN_NO_ERROR;
+        POP_AND_RETURN(SVN_NO_ERROR);
     }
 
   // Do some prep work for tranforming the DIFF parameter into a
@@ -83,7 +84,7 @@ DiffSummaryReceiver::onSummary(const svn
   static jmethodID ctor = 0;
   clazz = env->FindClass(JAVA_PACKAGE "/DiffSummary");
   if (JNIUtil::isJavaExceptionThrown())
-    return SVN_NO_ERROR;
+    POP_AND_RETURN(SVN_NO_ERROR);
 
   if (ctor == 0)
     {
@@ -92,51 +93,32 @@ DiffSummaryReceiver::onSummary(const svn
                               "L"JAVA_PACKAGE"/DiffSummary$DiffKind;Z"
                               "L"JAVA_PACKAGE"/NodeKind;)V");
       if (JNIUtil::isJavaExceptionThrown() || ctor == 0)
-        return SVN_NO_ERROR;
+        POP_AND_RETURN(SVN_NO_ERROR);
     }
   // Convert the arguments into their Java equivalent,
   jstring jPath = JNIUtil::makeJString(diff->path);
   if (JNIUtil::isJavaExceptionThrown())
-    return SVN_NO_ERROR;
+    POP_AND_RETURN(SVN_NO_ERROR);
 
   jobject jNodeKind = EnumMapper::mapNodeKind(diff->node_kind);
   if (JNIUtil::isJavaExceptionThrown())
-    return SVN_NO_ERROR;
+    POP_AND_RETURN(SVN_NO_ERROR);
 
   jobject jSummarizeKind = EnumMapper::mapSummarizeKind(diff->summarize_kind);
   if (JNIUtil::isJavaExceptionThrown())
-    return SVN_NO_ERROR;
+    POP_AND_RETURN(SVN_NO_ERROR);
 
   // Actually tranform the DIFF parameter into a Java equivalent.
   jobject jDiffSummary = env->NewObject(clazz, ctor, jPath, jSummarizeKind,
                                         (jboolean) diff->prop_changed,
                                         jNodeKind);
   if (JNIUtil::isJavaExceptionThrown() || jDiffSummary == NULL)
-    return SVN_NO_ERROR;
-
-  env->DeleteLocalRef(jSummarizeKind);
-  if (JNIUtil::isJavaExceptionThrown())
-    return SVN_NO_ERROR;
-
-  env->DeleteLocalRef(jPath);
-  if (JNIUtil::isJavaExceptionThrown())
-    return SVN_NO_ERROR;
-
-  env->DeleteLocalRef(jNodeKind);
-  if (JNIUtil::isJavaExceptionThrown())
-    return SVN_NO_ERROR;
-
-  env->DeleteLocalRef(clazz);
-  if (JNIUtil::isJavaExceptionThrown())
-    return SVN_NO_ERROR;
+    POP_AND_RETURN(SVN_NO_ERROR);
 
   // Invoke the Java DiffSummaryReceiver callback.
   env->CallVoidMethod(m_receiver, callback, jDiffSummary);
-  if (JNIUtil::isJavaExceptionThrown())
-    return SVN_NO_ERROR;
-
-  env->DeleteLocalRef(jDiffSummary);
   // We return whether an exception was thrown or not.
 
+  env->PopLocalFrame(NULL);
   return SVN_NO_ERROR;
 }

Modified: subversion/trunk/subversion/bindings/javahl/native/JNIUtil.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/javahl/native/JNIUtil.h?rev=925387&r1=925386&r2=925387&view=diff
==============================================================================
--- subversion/trunk/subversion/bindings/javahl/native/JNIUtil.h (original)
+++ subversion/trunk/subversion/bindings/javahl/native/JNIUtil.h Fri Mar 19 19:22:41 2010
@@ -244,4 +244,25 @@ class JNIUtil
     }                                                   \
   } while (0)
 
+/**
+ * The initial capacity of a create local reference frame.
+ */
+#define LOCAL_FRAME_SIZE            16
+
+/**
+ * A statement macro use to pop the reference frame and return NULL
+ */
+#define POP_AND_RETURN(ret_val)         \
+  do                                    \
+    {                                   \
+      env->PopLocalFrame(NULL);         \
+      return (ret_val);                 \
+    }                                   \
+  while (0)
+
+/**
+ * A useful macro.
+ */
+#define POP_AND_RETURN_NULL             POP_AND_RETURN(NULL)
+
 #endif  // JNIUTIL_H

Modified: subversion/trunk/subversion/bindings/javahl/native/ListCallback.cpp
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/javahl/native/ListCallback.cpp?rev=925387&r1=925386&r2=925387&view=diff
==============================================================================
--- subversion/trunk/subversion/bindings/javahl/native/ListCallback.cpp (original)
+++ subversion/trunk/subversion/bindings/javahl/native/ListCallback.cpp Fri Mar 19 19:22:41
2010
@@ -75,6 +75,11 @@ ListCallback::doList(const char *path,
 {
   JNIEnv *env = JNIUtil::getEnv();
 
+  // Create a local frame for our references
+  env->PushLocalFrame(LOCAL_FRAME_SIZE);
+  if (JNIUtil::isJavaExceptionThrown())
+    return SVN_NO_ERROR;
+
   // The method id will not change during the time this library is
   // loaded, so it can be cached.
   static jmethodID mid = 0;
@@ -82,45 +87,33 @@ ListCallback::doList(const char *path,
     {
       jclass clazz = env->FindClass(JAVA_PACKAGE"/callback/ListCallback");
       if (JNIUtil::isJavaExceptionThrown())
-        return SVN_NO_ERROR;
+        POP_AND_RETURN(SVN_NO_ERROR);
 
       mid = env->GetMethodID(clazz, "doEntry",
                              "(L"JAVA_PACKAGE"/DirEntry;"
                              "L"JAVA_PACKAGE"/Lock;)V");
       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);
     }
 
   // convert the parameters to their Java relatives
   jobject jdirentry = createJavaDirEntry(path, abs_path, dirent);
   if (JNIUtil::isJavaExceptionThrown())
-    return SVN_NO_ERROR;
+    POP_AND_RETURN(SVN_NO_ERROR);
 
-  jobject jlock;
+  jobject jlock = NULL;
   if (lock != NULL)
     {
       jlock = CreateJ::Lock(lock);
       if (JNIUtil::isJavaExceptionThrown())
-        return SVN_NO_ERROR;
-    }
-  else
-    {
-      jlock = NULL;
+        POP_AND_RETURN(SVN_NO_ERROR);
     }
 
   // call the Java method
   env->CallVoidMethod(m_callback, mid, jdirentry, jlock);
-  if (JNIUtil::isJavaExceptionThrown())
-    return SVN_NO_ERROR;
-
-  // cleanup the temporary Java objects
-  env->DeleteLocalRef(jdirentry);
   // No need to check for exception here, because we'll just return anyway
 
+  env->PopLocalFrame(NULL);
   return SVN_NO_ERROR;
 }
 

Modified: subversion/trunk/subversion/bindings/javahl/native/LogMessageCallback.cpp
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/javahl/native/LogMessageCallback.cpp?rev=925387&r1=925386&r2=925387&view=diff
==============================================================================
--- subversion/trunk/subversion/bindings/javahl/native/LogMessageCallback.cpp (original)
+++ subversion/trunk/subversion/bindings/javahl/native/LogMessageCallback.cpp Fri Mar 19 19:22:41
2010
@@ -70,6 +70,11 @@ LogMessageCallback::singleMessage(svn_lo
 {
   JNIEnv *env = JNIUtil::getEnv();
 
+  // Create a local frame for our references
+  env->PushLocalFrame(LOCAL_FRAME_SIZE);
+  if (JNIUtil::isJavaExceptionThrown())
+    return SVN_NO_ERROR;
+
   // The method id will not change during the time this library is
   // loaded, so it can be cached.
   static jmethodID sm_mid = 0;
@@ -77,22 +82,18 @@ LogMessageCallback::singleMessage(svn_lo
     {
       jclass clazz = env->FindClass(JAVA_PACKAGE"/callback/LogMessageCallback");
       if (JNIUtil::isJavaExceptionThrown())
-        return SVN_NO_ERROR;
+        POP_AND_RETURN(SVN_NO_ERROR);
 
       sm_mid = env->GetMethodID(clazz,
                                 "singleMessage",
                                 "(Ljava/util/Set;JLjava/util/Map;Z)V");
       if (JNIUtil::isJavaExceptionThrown())
-        return SVN_NO_ERROR;
-
-      env->DeleteLocalRef(clazz);
-      if (JNIUtil::isJavaExceptionThrown())
-        return SVN_NO_ERROR;
+        POP_AND_RETURN(SVN_NO_ERROR);
     }
 
   jclass clazzCP = env->FindClass(JAVA_PACKAGE"/ChangePath");
   if (JNIUtil::isJavaExceptionThrown())
-    return SVN_NO_ERROR;
+    POP_AND_RETURN(SVN_NO_ERROR);
 
   static jmethodID midCP = 0;
   if (midCP == 0)
@@ -104,7 +105,7 @@ LogMessageCallback::singleMessage(svn_lo
                                "L"JAVA_PACKAGE"/Tristate;"
                                "L"JAVA_PACKAGE"/Tristate;)V");
       if (JNIUtil::isJavaExceptionThrown())
-        return SVN_NO_ERROR;
+        POP_AND_RETURN(SVN_NO_ERROR);
     }
 
   jobject jChangedPaths = NULL;
@@ -123,15 +124,15 @@ LogMessageCallback::singleMessage(svn_lo
 
           jstring jpath = JNIUtil::makeJString(path);
           if (JNIUtil::isJavaExceptionThrown())
-            return SVN_NO_ERROR;
+            POP_AND_RETURN(SVN_NO_ERROR);
 
           jstring jcopyFromPath = JNIUtil::makeJString(log_item->copyfrom_path);
           if (JNIUtil::isJavaExceptionThrown())
-            return SVN_NO_ERROR;
+            POP_AND_RETURN(SVN_NO_ERROR);
 
           jobject jnodeKind = EnumMapper::mapNodeKind(log_item->node_kind);
           if (JNIUtil::isJavaExceptionThrown())
-            return SVN_NO_ERROR;
+            POP_AND_RETURN(SVN_NO_ERROR);
 
           jlong jcopyFromRev = log_item->copyfrom_rev;
           jchar jaction = log_item->action;
@@ -141,21 +142,21 @@ LogMessageCallback::singleMessage(svn_lo
                               EnumMapper::mapTristate(log_item->text_modified),
                               EnumMapper::mapTristate(log_item->props_modified));
           if (JNIUtil::isJavaExceptionThrown())
-            return SVN_NO_ERROR;
+            POP_AND_RETURN(SVN_NO_ERROR);
 
           jcps.push_back(cp);
 
           env->DeleteLocalRef(jnodeKind);
           if (JNIUtil::isJavaExceptionThrown())
-            return SVN_NO_ERROR;
+            POP_AND_RETURN(SVN_NO_ERROR);
 
           env->DeleteLocalRef(jpath);
           if (JNIUtil::isJavaExceptionThrown())
-            return SVN_NO_ERROR;
+            POP_AND_RETURN(SVN_NO_ERROR);
 
           env->DeleteLocalRef(jcopyFromPath);
           if (JNIUtil::isJavaExceptionThrown())
-            return SVN_NO_ERROR;
+            POP_AND_RETURN(SVN_NO_ERROR);
         }
 
       jChangedPaths = CreateJ::Set(jcps);
@@ -171,15 +172,8 @@ LogMessageCallback::singleMessage(svn_lo
                       (jlong)log_entry->revision,
                       jrevprops,
                       (jboolean)log_entry->has_children);
-  if (JNIUtil::isJavaExceptionThrown())
-    return SVN_NO_ERROR;
-
-  env->DeleteLocalRef(jChangedPaths);
-  if (JNIUtil::isJavaExceptionThrown())
-    return SVN_NO_ERROR;
-
-  env->DeleteLocalRef(jrevprops);
   // No need to check for an exception here, because we return anyway.
 
+  env->PopLocalFrame(NULL);
   return SVN_NO_ERROR;
 }

Modified: subversion/trunk/subversion/bindings/javahl/native/ProplistCallback.cpp
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/javahl/native/ProplistCallback.cpp?rev=925387&r1=925386&r2=925387&view=diff
==============================================================================
--- subversion/trunk/subversion/bindings/javahl/native/ProplistCallback.cpp (original)
+++ subversion/trunk/subversion/bindings/javahl/native/ProplistCallback.cpp Fri Mar 19 19:22:41
2010
@@ -71,6 +71,11 @@ svn_error_t *ProplistCallback::singlePat
 {
   JNIEnv *env = JNIUtil::getEnv();
 
+  // Create a local frame for our references
+  env->PushLocalFrame(LOCAL_FRAME_SIZE);
+  if (JNIUtil::isJavaExceptionThrown())
+    return NULL;
+
   // The method id will not change during the time this library is
   // loaded, so it can be cached.
   static jmethodID mid = 0;
@@ -83,35 +88,23 @@ svn_error_t *ProplistCallback::singlePat
       mid = env->GetMethodID(clazz, "singlePath",
                              "(Ljava/lang/String;Ljava/util/Map;)V");
       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);
     }
 
   // convert the parameters to their Java relatives
   jstring jpath = JNIUtil::makeJString(path);
   if (JNIUtil::isJavaExceptionThrown())
-    return SVN_NO_ERROR;
+    POP_AND_RETURN(SVN_NO_ERROR);
 
-  jobject jmap = NULL;
-  jmap = CreateJ::PropertyMap(prop_hash, pool);
+  jobject jmap = CreateJ::PropertyMap(prop_hash, pool);
   if (JNIUtil::isJavaExceptionThrown())
-    return SVN_NO_ERROR;
+    POP_AND_RETURN(SVN_NO_ERROR);
 
   // call the Java method
   env->CallVoidMethod(m_callback, mid, jpath, jmap);
-  if (JNIUtil::isJavaExceptionThrown())
-    return SVN_NO_ERROR;
-
-  // cleanup the temporary Java objects
-  env->DeleteLocalRef(jpath);
-  if (JNIUtil::isJavaExceptionThrown())
-    return SVN_NO_ERROR;
-
-  env->DeleteLocalRef(jmap);
   // We return whether an exception was thrown or not.
 
+  env->PopLocalFrame(NULL);
+
   return SVN_NO_ERROR;
 }



Mime
View raw message