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.strap...@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
