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.

-Marc

Reply via email to