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; }