On 29.04.2015 16:02, Branko Čibej wrote:
> On 29.04.2015 11:57, Marc Strapetz wrote:
>> On 29.04.2015 05:31, Branko Čibej wrote:
>>> On 28.04.2015 21:22, Bert Huijben wrote:
>>>>> -----Original Message-----
>>>>> From: Marc Strapetz [mailto:marc.strapetz_at_syntevo.com]
>>>>> Sent: dinsdag 28 april 2015 20:26
>>>>> To: Branko Čibej
>>>>> Cc: Subversion Development
>>>>> Subject: Re: 1.9 JavaHL memory leak in ISVNRemote#status
>>>>> Also, I should add that according to the Profiler, the byte[]s are
>>>>> referenced from the Checksums. The char[]s are referenced from the
>>>>> Strings. And the Strings are referenced directly as JNI local
>>>>> references. Browsing through these Strings, they seem to be
>>>>> server-side
>>>>> paths ("subversion/branches/1.8.x/...")
>>>> Just guessing: Notifications?
>>>
>>> No, this is an RA status edit drive; there are no notifications, only
>>> editor callbacks, and the checksum objects are created in in the
>>> callbacks related to file content changes (file contents streams and
>>> checksums always come in pairs).
>>>
>>> I counted creations, finalizations and garbage collections again. I
>>> added forced finalization and GC calls to the test case. For every loop
>>> in the test, we create 57 Checksum instances, but only one of them is
>>> finalized, no matter how often the finalizer and GC are run. All the
>>> Checksum objects are created in the same way, and here are /no/
>>> references anywhere to the remaining 56 objects, yet they're neither
>>> finalized nor garbage-collected. The fields (byte array and kind) /are/
>>> collected; all the "live" (according to the heap profiler) Checksum
>>> objects have their fields set to null.
>> I've been testing on Windows. According to JProfiler and JVisualVM,
>> byte[]s are still referenced from the Checksums. Hence, I would expect
>> that they are not garbage collected.
>>
>>> clearly, the code is cleaning up the references correctly
>> I don't have detailed understanding of the "jniwrapper" package, but I
>> tend to agree with you. In the native code, CreateJ::Checksum and
>> CreateJ::PropertyMap are basically doing the same thing, so there is
>> no reason why Checksums would remain referenced while HashMaps
>> properly do not.
>>
>> I've also tried to comment out all env.CallVoidMethod()-callbacks in
>> EditorProxy.cpp, so created object references would not even be passed
>> into the Java code. Still the same, Checksums remain as "JNI local
>> reference".
>>
>> Finally, I've tried to explicitly call DeleteLocalRef(). This /solves/
>> the memory leak (at least for Checksums), but I don't understand why
>> this is necessary and whether this is correct.
>>
>> svn_error_t*
>> EditorProxy::cb_alter_file(void *baton,
>> const char *relpath,
>> ...
>> jstring jrelpath = JNIUtil::makeJString(relpath);
>> SVN_JAVAHL_OLDSTYLE_EXCEPTION_CHECK(env);
>> jobject jchecksum = CreateJ::Checksum(checksum);
>> SVN_JAVAHL_OLDSTYLE_EXCEPTION_CHECK(env);
>> jobject jprops = CreateJ::PropertyMap(props, scratch_pool);
>> SVN_JAVAHL_OLDSTYLE_EXCEPTION_CHECK(env);
>>
>> jobject jcontents = NULL;
>> if (contents != NULL)
>> jcontents = wrap_input_stream(contents);
>>
>> env.CallVoidMethod(ep->m_jeditor, mid,
>> jrelpath, jlong(revision),
>> jchecksum, jcontents, jprops);
>>
>> env.DeleteLocalRef(jrelpath);
>> env.DeleteLocalRef(jchecksum);
>> env.DeleteLocalRef(jprops);
>> if (contents != NULL)
>> env.DeleteLocalRef(jcontents);
>> ...
>>
>>> but for some unfathomable reason, the collector keeps them
>>> alive for a while.
>> I'm not entirely sure about the exact difference of the live data in
>> the VM and a heap dump, but IMO the Checksums are still considered as
>> referenced ("JNI local reference") and hence will never be garbage
>> collected. The profilers confirm this.
>>
>> Given that DeleteLocalRef solves the problem, I think this is either a
>> bug in the jniwrapper or a bug in JNI itself.
> The latest code wraps the callback implementations with
> PushLocalFrame/PopLocalFrame; any references created within a local
> frame should be automatically deleted by PopLocalFrame, according to all
> JNI docs I can find.
>
> I can add the explicit deletions, but it's a shame that frame management
> wouldn't work as expected. :( So, I'm going to double-check if we're
> actually getting the frame management right. I can't imagine why the
> HashMaps and NativeInputStreams would be released, but not the Checksums.
>
> All in all, I agree with you that this looks like a JNI bug ... the
> trick now will be to prove that with a minimal test case and report it
> upstream. :)
>
> (FWIW, I'm using Java 8u45 64-bit on OSX.)
So, interesting data point ... I moved the creation of the Checksum
objects after the creation of the property maps ... and now they're
getting garbage-collected. This is becoming extremely weird.
-- Brane
Received on 2015-04-29 17:03:56 CEST