You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Blair Zajac <bl...@orcaware.com> on 2011/09/28 06:27:50 UTC

Re: svn commit: r1176416 - /subversion/trunk/subversion/bindings/javahl/native/JNIUtil.cpp

On Sep 27, 2011, at 7:56 AM, hwright@apache.org wrote:

> Author: hwright
> Date: Tue Sep 27 14:56:56 2011
> New Revision: 1176416
> 
> 
> Modified: subversion/trunk/subversion/bindings/javahl/native/JNIUtil.cpp
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/javahl/native/JNIUtil.cpp?rev=1176416&r1=1176415&r2=1176416&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/bindings/javahl/native/JNIUtil.cpp (original)
> +++ subversion/trunk/subversion/bindings/javahl/native/JNIUtil.cpp Tue Sep 27 14:56:56 2011
> @@ -474,9 +474,9 @@ void JNIUtil::handleSVNError(svn_error_t
>       if (isJavaExceptionThrown())
>         POP_AND_RETURN_NOTHING();
>     }
> -  Array stackTraceArray((jobjectArray) env->CallObjectMethod(nativeException,
> -                                                             mid_gst));
> -  std::vector<jobject> oldStackTrace = stackTraceArray.vector();
> +  Array *stackTraceArray =
> +    new Array((jobjectArray) env->CallObjectMethod(nativeException, mid_gst));
> +  std::vector<jobject> oldStackTrace = stackTraceArray->vector();
> 
>   // Build the new stack trace elements from the chained errors.
>   std::vector<jobject> newStackTrace;
> @@ -506,6 +506,8 @@ void JNIUtil::handleSVNError(svn_error_t
>       ++i;
>     }
> 
> +  delete stackTraceArray;

Hi Hyrum,

Out of curiosity, since I'm not familiar with JNI coding, is there a chance of memory leak here if an exception is thrown before the delete?  Or is using something like std::auto_ptr a good idea?

Blair


Re: svn commit: r1176416 - /subversion/trunk/subversion/bindings/javahl/native/JNIUtil.cpp

Posted by Hyrum K Wright <hy...@wandisco.com>.
On Tue, Sep 27, 2011 at 11:27 PM, Blair Zajac <bl...@orcaware.com> wrote:
>
> On Sep 27, 2011, at 7:56 AM, hwright@apache.org wrote:
>
>> Author: hwright
>> Date: Tue Sep 27 14:56:56 2011
>> New Revision: 1176416
>>
>>
>> Modified: subversion/trunk/subversion/bindings/javahl/native/JNIUtil.cpp
>> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/javahl/native/JNIUtil.cpp?rev=1176416&r1=1176415&r2=1176416&view=diff
>> ==============================================================================
>> --- subversion/trunk/subversion/bindings/javahl/native/JNIUtil.cpp (original)
>> +++ subversion/trunk/subversion/bindings/javahl/native/JNIUtil.cpp Tue Sep 27 14:56:56 2011
>> @@ -474,9 +474,9 @@ void JNIUtil::handleSVNError(svn_error_t
>>       if (isJavaExceptionThrown())
>>         POP_AND_RETURN_NOTHING();
>>     }
>> -  Array stackTraceArray((jobjectArray) env->CallObjectMethod(nativeException,
>> -                                                             mid_gst));
>> -  std::vector<jobject> oldStackTrace = stackTraceArray.vector();
>> +  Array *stackTraceArray =
>> +    new Array((jobjectArray) env->CallObjectMethod(nativeException, mid_gst));
>> +  std::vector<jobject> oldStackTrace = stackTraceArray->vector();
>>
>>   // Build the new stack trace elements from the chained errors.
>>   std::vector<jobject> newStackTrace;
>> @@ -506,6 +506,8 @@ void JNIUtil::handleSVNError(svn_error_t
>>       ++i;
>>     }
>>
>> +  delete stackTraceArray;
>
> Hi Hyrum,
>
> Out of curiosity, since I'm not familiar with JNI coding, is there a chance of memory leak here if an exception is thrown before the delete?  Or is using something like std::auto_ptr a good idea?

Blair,
There is indeed a leak, but it's only a leak of the C++ object, and
not the underlying Java objects.  For a long and tortured discussion
Philip and I had on IRC about this subject yesterday, see:
http://colabti.org/irclogger/irclogger_log/svn-dev?date=2011-09-27#l100

The basic gist is that the JVM automagically releases local references
when existing a JNI call, so the simplest solution may be to
temporarily leak those references from within the Array proxy object.

-Hyrum


-- 

uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com/