On 28.04.2015 18:03, Marc Strapetz wrote:
> Hi Brane,
>
> On 28.04.2015 07:36, Branko Čibej wrote:
>> On 24.04.2015 14:11, Branko Čibej wrote:
>>>
>>> Hi Marc,
>>>
>>> Just a quick note: your last msg jogged my memory and I think I know
>>> the root cause of the leak: improper JNI frame management within a
>>> loop. If I'm right, I can both fix the leak and remove the
>>> close-stream requirement I just added.
>>>
>>> On 24 Apr 2015 11:00 am, "Marc Strapetz" <[email protected]
>>> <mailto:[email protected]>> wrote:
>>>
>>> On 24.04.2015 06 <tel:24.04.2015%2006>:34, Branko Čibej wrote:
>>>
>>> On 22.03.2015 05 <tel:22.03.2015%2005>:06, Branko Čibej wrote:
>>>
>>> On 21.03.2015 16 <tel:21.03.2015%2016>:23, Branko Čibej
>>> wrote:
>>>
>>> On 19.03.2015 11:43, Marc Strapetz wrote:
>>>
>>> Attached example performs an endless series of
>>> remote status against
>>> the Subversion repository. When invoked with
>>> -Xmx24M, the VM will run
>>> out of memory soon. Monitoring with jvisualvm
>>> shows that the used heap
>>> size constantly grows. Monitoring with the Task
>>> Manager shows that the
>>> allocated memory grows even more (significantly).
>>> Looks like a memory
>>> leak, for which a large amount of native memory is
>>> involved, too.
>>>
>>> Tested on Windows 8.1 with almost latest
>>> Subversion 1.9 JavaHL builds.
>>>
>>> I can confirm that this happens on the Mac, too, and
>>> it's not a garbage
>>> collector artefact. I'm trying to trace where the leak
>>> is happening ...
>>> valgrind with APR pool debugging doesn't tell me much
>>> (no surprise there).
>>>
>>> Just to make sure we weren't doing something bad in our
>>> libraries, I
>>> wrote a small C program that does the same as your Java
>>> example (Ev2
>>> shims included), and memory usage is completely steady. So
>>> it is
>>> something in JavaHL, but I have no clue yet what the
>>> problem is.
>>>
>>>
>>> I have to say this was one of the more "interesting" bug-hunts
>>> in my not
>>> entirely boring career, and that's not really obvious from
>>> the fix
>>> itself. :)
>>>
>>> http://svn.apache.org/r1675771
>>>
>>> Marc: this will not be in RC1, but please give the patch a
>>> spin and let
>>> me know if it fixes your problem. I tested this with the Java
>>> program
>>> you attached to your original report, and heap size no longer
>>> grows
>>> without bounds.
>>>
>>>
>>> Great hunt, Brane! The native leak seems to be fixed. I've run my
>>> remote status loop with -Xmx24M and still get an OOME after ~170
>>> loop iterations. The memory leak is significantly smaller and this
>>> time it seems to be in the Java part. According to the profiler,
>>> most memory is allocated by HashMap and friends, referenced from
>>> JNI code. Only two org.apache.subversion classes show up, but I
>>> guess they indicate the source of the leak:
>>>
>>> org.apache.subversion.javahl.types.Checksum (~10K instances)
>>> org.apache.subversion.javahl.types.NativeInputStream (~10K
>>> instances)
>>>
>>> Let me know, if you more profiler statistics will be helpful.
>>>
>>
>> So I've been looking at this in depth. At first I thought that one of
>> the problems was that we didn't release JNI local references; I added
>> code to make sure this happens in the status callbacks (not committed
>> yet) and I verified that all the native wrapped objects do get
>> finalized. However, the Java objects still hang around.
>>
>> One of the problems is that all the callbacks happen within the scope of
>> the ISVNReporter.finishReport call, which means that the whole edit
>> drive is considered a single JNI call (despite the callbacks to Java)
>> and the garbage collector can't reclaim space for the objects created
>> within JNI during that time. But even a forced GC after the report is
>> done and the remote session disposed won't release all the native
>> references. I'm a bit stumped here ... JVM's built-in memory profiler
>> shows the live references and where they're allocated, but doesn't show
>> why they're not released even when I explicitly create and destroy JNI
>> frames.
>
> Can you please commit your current state somewhere or send me a patch?
> I can give this one a try in JProfiler and see whether I can gather
> some more useful information.
Here's the complete patch against 1.9.x. I do see some NativeInputStream
objects (but not all) being garbage-collected, but there are a number of
other objects, even those allocated in Java code in the callback, that
are just hanging around, even if I force a GC. Any help in tracking this
down will be greatly appreciated.
-- Brane
Index: subversion/bindings/javahl/native/EditorProxy.cpp
===================================================================
--- subversion/bindings/javahl/native/EditorProxy.cpp (revision 1676575)
+++ subversion/bindings/javahl/native/EditorProxy.cpp (working copy)
@@ -151,29 +151,32 @@
apr_pool_t *scratch_pool)
{
//DEBUG:fprintf(stderr, " (n) EditorProxy::cb_add_directory('%s')\n",
relpath);
+ const ::Java::Env env;
+ SVN_JAVAHL_CATCH(env, SVN_ERR_RA_SVN_EDIT_ABORTED,
+ {
+ ::Java::LocalFrame frame(env);
- EditorProxy* const ep = static_cast<EditorProxy*>(baton);
- if (!ep->m_valid)
- return invalid_editor();
+ EditorProxy* const ep = static_cast<EditorProxy*>(baton);
+ if (!ep->m_valid)
+ return invalid_editor();
- static jmethodID mid = 0;
- SVN_ERR(get_editor_method(mid, "addDirectory",
- "(Ljava/lang/String;"
- "Ljava/lang/Iterable;"
- "Ljava/util/Map;J)V"));
+ static jmethodID mid = 0;
+ SVN_ERR(get_editor_method(mid, "addDirectory",
+ "(Ljava/lang/String;"
+ "Ljava/lang/Iterable;"
+ "Ljava/util/Map;J)V"));
- jstring jrelpath = JNIUtil::makeJString(relpath);
- SVN_JNI_CATCH(,SVN_ERR_RA_SVN_EDIT_ABORTED);
- jobject jchildren = (!children ? NULL : CreateJ::StringSet(children));
- SVN_JNI_CATCH(,SVN_ERR_RA_SVN_EDIT_ABORTED);
- jobject jprops = CreateJ::PropertyMap(props, scratch_pool);
- SVN_JNI_CATCH(,SVN_ERR_RA_SVN_EDIT_ABORTED);
+ jstring jrelpath = JNIUtil::makeJString(relpath);
+ SVN_JAVAHL_OLDSTYLE_EXCEPTION_CHECK(env);
+ jobject jchildren = (!children ? NULL : CreateJ::StringSet(children));
+ SVN_JAVAHL_OLDSTYLE_EXCEPTION_CHECK(env);
+ jobject jprops = CreateJ::PropertyMap(props, scratch_pool);
+ SVN_JAVAHL_OLDSTYLE_EXCEPTION_CHECK(env);
- SVN_JNI_CATCH(
- JNIUtil::getEnv()->CallVoidMethod(ep->m_jeditor, mid,
- jrelpath, jchildren, jprops,
- jlong(replaces_rev)),
- SVN_ERR_RA_SVN_EDIT_ABORTED);
+ env.CallVoidMethod(ep->m_jeditor, mid,
+ jrelpath, jchildren, jprops,
+ jlong(replaces_rev));
+ });
return SVN_NO_ERROR;
}
@@ -187,36 +190,37 @@
apr_pool_t *scratch_pool)
{
//DEBUG:fprintf(stderr, " (n) EditorProxy::cb_add_file('%s')\n", relpath);
+ const ::Java::Env env;
+ SVN_JAVAHL_CATCH(env, SVN_ERR_RA_SVN_EDIT_ABORTED,
+ {
+ ::Java::LocalFrame frame(env);
- EditorProxy* const ep = static_cast<EditorProxy*>(baton);
- if (!ep || !ep->m_valid)
- return invalid_editor();
+ EditorProxy* const ep = static_cast<EditorProxy*>(baton);
+ if (!ep || !ep->m_valid)
+ return invalid_editor();
- static jmethodID mid = 0;
- SVN_ERR(get_editor_method(mid, "addFile",
- "(Ljava/lang/String;"
- "L"JAVA_PACKAGE"/types/Checksum;"
- "Ljava/io/InputStream;"
- "Ljava/util/Map;J)V"));
+ static jmethodID mid = 0;
+ SVN_ERR(get_editor_method(mid, "addFile",
+ "(Ljava/lang/String;"
+ "L"JAVA_PACKAGE"/types/Checksum;"
+ "Ljava/io/InputStream;"
+ "Ljava/util/Map;J)V"));
- jstring jrelpath = JNIUtil::makeJString(relpath);
- SVN_JNI_CATCH(,SVN_ERR_RA_SVN_EDIT_ABORTED);
- jobject jchecksum = CreateJ::Checksum(checksum);
- SVN_JNI_CATCH(,SVN_ERR_RA_SVN_EDIT_ABORTED);
- jobject jcontents = NULL;
- SVN_JNI_CATCH(,SVN_ERR_RA_SVN_EDIT_ABORTED);
- jobject jprops = CreateJ::PropertyMap(props, scratch_pool);
- SVN_JNI_CATCH(,SVN_ERR_RA_SVN_EDIT_ABORTED);
+ 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);
- if (contents != NULL)
- SVN_JAVAHL_CATCH(Java::Env(), SVN_ERR_RA_SVN_EDIT_ABORTED,
- jcontents = wrap_input_stream(contents));
+ jobject jcontents = NULL;
+ if (contents != NULL)
+ jcontents = wrap_input_stream(contents);
- SVN_JNI_CATCH(
- JNIUtil::getEnv()->CallVoidMethod(ep->m_jeditor, mid,
- jrelpath, jchecksum, jcontents,
- jprops, jlong(replaces_rev)),
- SVN_ERR_RA_SVN_EDIT_ABORTED);
+ env.CallVoidMethod(ep->m_jeditor, mid,
+ jrelpath, jchecksum, jcontents,
+ jprops, jlong(replaces_rev));
+ });
return SVN_NO_ERROR;
}
@@ -229,29 +233,32 @@
apr_pool_t *scratch_pool)
{
//DEBUG:fprintf(stderr, " (n) EditorProxy::cb_add_symlink('%s')\n",
relpath);
+ const ::Java::Env env;
+ SVN_JAVAHL_CATCH(env, SVN_ERR_RA_SVN_EDIT_ABORTED,
+ {
+ ::Java::LocalFrame frame(env);
- EditorProxy* const ep = static_cast<EditorProxy*>(baton);
- if (!ep || !ep->m_valid)
- return invalid_editor();
+ EditorProxy* const ep = static_cast<EditorProxy*>(baton);
+ if (!ep || !ep->m_valid)
+ return invalid_editor();
- static jmethodID mid = 0;
- SVN_ERR(get_editor_method(mid, "addSymlink",
- "(Ljava/lang/String;"
- "Ljava/lang/String;"
- "Ljava/util/Map;J)V"));
+ static jmethodID mid = 0;
+ SVN_ERR(get_editor_method(mid, "addSymlink",
+ "(Ljava/lang/String;"
+ "Ljava/lang/String;"
+ "Ljava/util/Map;J)V"));
- jstring jrelpath = JNIUtil::makeJString(relpath);
- SVN_JNI_CATCH(,SVN_ERR_RA_SVN_EDIT_ABORTED);
- jstring jtarget = JNIUtil::makeJString(target);
- SVN_JNI_CATCH(,SVN_ERR_RA_SVN_EDIT_ABORTED);
- jobject jprops = CreateJ::PropertyMap(props, scratch_pool);
- SVN_JNI_CATCH(,SVN_ERR_RA_SVN_EDIT_ABORTED);
+ jstring jrelpath = JNIUtil::makeJString(relpath);
+ SVN_JAVAHL_OLDSTYLE_EXCEPTION_CHECK(env);
+ jstring jtarget = JNIUtil::makeJString(target);
+ SVN_JAVAHL_OLDSTYLE_EXCEPTION_CHECK(env);
+ jobject jprops = CreateJ::PropertyMap(props, scratch_pool);
+ SVN_JAVAHL_OLDSTYLE_EXCEPTION_CHECK(env);
- SVN_JNI_CATCH(
- JNIUtil::getEnv()->CallVoidMethod(ep->m_jeditor, mid,
- jrelpath, jtarget, jprops,
- jlong(replaces_rev)),
- SVN_ERR_RA_SVN_EDIT_ABORTED);
+ env.CallVoidMethod(ep->m_jeditor, mid,
+ jrelpath, jtarget, jprops,
+ jlong(replaces_rev));
+ });
return SVN_NO_ERROR;
}
@@ -263,27 +270,30 @@
apr_pool_t *scratch_pool)
{
//DEBUG:fprintf(stderr, " (n) EditorProxy::cb_add_absent('%s')\n", relpath);
+ const ::Java::Env env;
+ SVN_JAVAHL_CATCH(env, SVN_ERR_RA_SVN_EDIT_ABORTED,
+ {
+ ::Java::LocalFrame frame(env);
- EditorProxy* const ep = static_cast<EditorProxy*>(baton);
- if (!ep || !ep->m_valid)
- return invalid_editor();
+ EditorProxy* const ep = static_cast<EditorProxy*>(baton);
+ if (!ep || !ep->m_valid)
+ return invalid_editor();
- static jmethodID mid = 0;
- SVN_ERR(get_editor_method(mid, "addAbsent",
- "(Ljava/lang/String;"
- "L"JAVA_PACKAGE"/types/NodeKind;"
- "J)V"));
+ static jmethodID mid = 0;
+ SVN_ERR(get_editor_method(mid, "addAbsent",
+ "(Ljava/lang/String;"
+ "L"JAVA_PACKAGE"/types/NodeKind;"
+ "J)V"));
- jstring jrelpath = JNIUtil::makeJString(relpath);
- SVN_JNI_CATCH(,SVN_ERR_RA_SVN_EDIT_ABORTED);
- jobject jkind = EnumMapper::mapNodeKind(kind);
- SVN_JNI_CATCH(,SVN_ERR_RA_SVN_EDIT_ABORTED);
+ jstring jrelpath = JNIUtil::makeJString(relpath);
+ SVN_JAVAHL_OLDSTYLE_EXCEPTION_CHECK(env);
+ jobject jkind = EnumMapper::mapNodeKind(kind);
+ SVN_JAVAHL_OLDSTYLE_EXCEPTION_CHECK(env);
- SVN_JNI_CATCH(
- JNIUtil::getEnv()->CallVoidMethod(ep->m_jeditor, mid,
- jrelpath, jkind,
- jlong(replaces_rev)),
- SVN_ERR_RA_SVN_EDIT_ABORTED);
+ env.CallVoidMethod(ep->m_jeditor, mid,
+ jrelpath, jkind,
+ jlong(replaces_rev));
+ });
return SVN_NO_ERROR;
}
@@ -297,29 +307,32 @@
{
//DEBUG:fprintf(stderr, " (n) EditorProxy::cb_alter_directory('%s',
r%lld)\n",
//DEBUG: relpath, static_cast<long long>(revision));
+ const ::Java::Env env;
+ SVN_JAVAHL_CATCH(env, SVN_ERR_RA_SVN_EDIT_ABORTED,
+ {
+ ::Java::LocalFrame frame(env);
- EditorProxy* const ep = static_cast<EditorProxy*>(baton);
- if (!ep || !ep->m_valid)
- return invalid_editor();
+ EditorProxy* const ep = static_cast<EditorProxy*>(baton);
+ if (!ep || !ep->m_valid)
+ return invalid_editor();
- static jmethodID mid = 0;
- SVN_ERR(get_editor_method(mid, "alterDirectory",
- "(Ljava/lang/String;J"
- "Ljava/lang/Iterable;"
- "Ljava/util/Map;)V"));
+ static jmethodID mid = 0;
+ SVN_ERR(get_editor_method(mid, "alterDirectory",
+ "(Ljava/lang/String;J"
+ "Ljava/lang/Iterable;"
+ "Ljava/util/Map;)V"));
- jstring jrelpath = JNIUtil::makeJString(relpath);
- SVN_JNI_CATCH(,SVN_ERR_RA_SVN_EDIT_ABORTED);
- jobject jchildren = (!children ? NULL : CreateJ::StringSet(children));
- SVN_JNI_CATCH(,SVN_ERR_RA_SVN_EDIT_ABORTED);
- jobject jprops = CreateJ::PropertyMap(props, scratch_pool);
- SVN_JNI_CATCH(,SVN_ERR_RA_SVN_EDIT_ABORTED);
+ jstring jrelpath = JNIUtil::makeJString(relpath);
+ SVN_JAVAHL_OLDSTYLE_EXCEPTION_CHECK(env);
+ jobject jchildren = (!children ? NULL : CreateJ::StringSet(children));
+ SVN_JAVAHL_OLDSTYLE_EXCEPTION_CHECK(env);
+ jobject jprops = CreateJ::PropertyMap(props, scratch_pool);
+ SVN_JAVAHL_OLDSTYLE_EXCEPTION_CHECK(env);
- SVN_JNI_CATCH(
- JNIUtil::getEnv()->CallVoidMethod(ep->m_jeditor, mid,
- jrelpath, jlong(revision),
- jchildren, jprops),
- SVN_ERR_RA_SVN_EDIT_ABORTED);
+ env.CallVoidMethod(ep->m_jeditor, mid,
+ jrelpath, jlong(revision),
+ jchildren, jprops);
+ });
return SVN_NO_ERROR;
}
@@ -334,36 +347,37 @@
{
//DEBUG:fprintf(stderr, " (n) EditorProxy::cb_alter_file('%s', r%lld)\n",
//DEBUG: relpath, static_cast<long long>(revision));
+ const ::Java::Env env;
+ SVN_JAVAHL_CATCH(env, SVN_ERR_RA_SVN_EDIT_ABORTED,
+ {
+ ::Java::LocalFrame frame(env);
- EditorProxy* const ep = static_cast<EditorProxy*>(baton);
- if (!ep || !ep->m_valid)
- return invalid_editor();
+ EditorProxy* const ep = static_cast<EditorProxy*>(baton);
+ if (!ep || !ep->m_valid)
+ return invalid_editor();
- static jmethodID mid = 0;
- SVN_ERR(get_editor_method(mid, "alterFile",
- "(Ljava/lang/String;J"
- "L"JAVA_PACKAGE"/types/Checksum;"
- "Ljava/io/InputStream;"
- "Ljava/util/Map;)V"));
+ static jmethodID mid = 0;
+ SVN_ERR(get_editor_method(mid, "alterFile",
+ "(Ljava/lang/String;J"
+ "L"JAVA_PACKAGE"/types/Checksum;"
+ "Ljava/io/InputStream;"
+ "Ljava/util/Map;)V"));
- jstring jrelpath = JNIUtil::makeJString(relpath);
- SVN_JNI_CATCH(,SVN_ERR_RA_SVN_EDIT_ABORTED);
- jobject jchecksum = CreateJ::Checksum(checksum);
- SVN_JNI_CATCH(,SVN_ERR_RA_SVN_EDIT_ABORTED);
- jobject jcontents = NULL;
- SVN_JNI_CATCH(,SVN_ERR_RA_SVN_EDIT_ABORTED);
- jobject jprops = CreateJ::PropertyMap(props, scratch_pool);
- SVN_JNI_CATCH(,SVN_ERR_RA_SVN_EDIT_ABORTED);
+ 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);
- if (contents != NULL)
- SVN_JAVAHL_CATCH(Java::Env(), SVN_ERR_RA_SVN_EDIT_ABORTED,
- jcontents = wrap_input_stream(contents));
+ jobject jcontents = NULL;
+ if (contents != NULL)
+ jcontents = wrap_input_stream(contents);
- SVN_JNI_CATCH(
- JNIUtil::getEnv()->CallVoidMethod(ep->m_jeditor, mid,
- jrelpath, jlong(revision),
- jchecksum, jcontents, jprops),
- SVN_ERR_RA_SVN_EDIT_ABORTED);
+ env.CallVoidMethod(ep->m_jeditor, mid,
+ jrelpath, jlong(revision),
+ jchecksum, jcontents, jprops);
+ });
return SVN_NO_ERROR;
}
@@ -377,29 +391,32 @@
{
//DEBUG:fprintf(stderr, " (n) EditorProxy::cb_alter_symlink('%s', r%lld)\n",
//DEBUG: relpath, static_cast<long long>(revision));
+ const ::Java::Env env;
+ SVN_JAVAHL_CATCH(env, SVN_ERR_RA_SVN_EDIT_ABORTED,
+ {
+ ::Java::LocalFrame frame(env);
- EditorProxy* const ep = static_cast<EditorProxy*>(baton);
- if (!ep || !ep->m_valid)
- return invalid_editor();
+ EditorProxy* const ep = static_cast<EditorProxy*>(baton);
+ if (!ep || !ep->m_valid)
+ return invalid_editor();
- static jmethodID mid = 0;
- SVN_ERR(get_editor_method(mid, "alterSymlink",
- "(Ljava/lang/String;J"
- "Ljava/lang/String;"
- "Ljava/util/Map;)V"));
+ static jmethodID mid = 0;
+ SVN_ERR(get_editor_method(mid, "alterSymlink",
+ "(Ljava/lang/String;J"
+ "Ljava/lang/String;"
+ "Ljava/util/Map;)V"));
- jstring jrelpath = JNIUtil::makeJString(relpath);
- SVN_JNI_CATCH(,SVN_ERR_RA_SVN_EDIT_ABORTED);
- jstring jtarget = JNIUtil::makeJString(target);
- SVN_JNI_CATCH(,SVN_ERR_RA_SVN_EDIT_ABORTED);
- jobject jprops = CreateJ::PropertyMap(props, scratch_pool);
- SVN_JNI_CATCH(,SVN_ERR_RA_SVN_EDIT_ABORTED);
+ jstring jrelpath = JNIUtil::makeJString(relpath);
+ SVN_JAVAHL_OLDSTYLE_EXCEPTION_CHECK(env);
+ jstring jtarget = JNIUtil::makeJString(target);
+ SVN_JAVAHL_OLDSTYLE_EXCEPTION_CHECK(env);
+ jobject jprops = CreateJ::PropertyMap(props, scratch_pool);
+ SVN_JAVAHL_OLDSTYLE_EXCEPTION_CHECK(env);
- SVN_JNI_CATCH(
- JNIUtil::getEnv()->CallVoidMethod(ep->m_jeditor, mid,
- jrelpath, jlong(revision),
- jtarget, jprops),
- SVN_ERR_RA_SVN_EDIT_ABORTED);
+ env.CallVoidMethod(ep->m_jeditor, mid,
+ jrelpath, jlong(revision),
+ jtarget, jprops);
+ });
return SVN_NO_ERROR;
}
@@ -411,21 +428,24 @@
{
//DEBUG:fprintf(stderr, " (n) EditorProxy::cb_delete('%s', r%lld)\n",
//DEBUG: relpath, static_cast<long long>(revision));
+ const ::Java::Env env;
+ SVN_JAVAHL_CATCH(env, SVN_ERR_RA_SVN_EDIT_ABORTED,
+ {
+ ::Java::LocalFrame frame(env);
- EditorProxy* const ep = static_cast<EditorProxy*>(baton);
- if (!ep || !ep->m_valid)
- return invalid_editor();
+ EditorProxy* const ep = static_cast<EditorProxy*>(baton);
+ if (!ep || !ep->m_valid)
+ return invalid_editor();
- static jmethodID mid = 0;
- SVN_ERR(get_editor_method(mid, "delete",
- "(Ljava/lang/String;J)V"));
+ static jmethodID mid = 0;
+ SVN_ERR(get_editor_method(mid, "delete",
+ "(Ljava/lang/String;J)V"));
- jstring jrelpath = JNIUtil::makeJString(relpath);
- SVN_JNI_CATCH(,SVN_ERR_RA_SVN_EDIT_ABORTED);
+ jstring jrelpath = JNIUtil::makeJString(relpath);
+ SVN_JAVAHL_OLDSTYLE_EXCEPTION_CHECK(env);
- SVN_JNI_CATCH(
- JNIUtil::getEnv()->CallVoidMethod(ep->m_jeditor, mid, jrelpath),
- SVN_ERR_RA_SVN_EDIT_ABORTED);
+ env.CallVoidMethod(ep->m_jeditor, mid, jrelpath);
+ });
return SVN_NO_ERROR;
}
@@ -439,26 +459,29 @@
{
//DEBUG:fprintf(stderr, " (n) EditorProxy::cb_copy('%s', r%lld, '%s')\n",
//DEBUG: src_relpath, static_cast<long long>(src_revision),
dst_relpath);
+ const ::Java::Env env;
+ SVN_JAVAHL_CATCH(env, SVN_ERR_RA_SVN_EDIT_ABORTED,
+ {
+ ::Java::LocalFrame frame(env);
- EditorProxy* const ep = static_cast<EditorProxy*>(baton);
- if (!ep || !ep->m_valid)
- return invalid_editor();
+ EditorProxy* const ep = static_cast<EditorProxy*>(baton);
+ if (!ep || !ep->m_valid)
+ return invalid_editor();
- static jmethodID mid = 0;
- SVN_ERR(get_editor_method(mid, "copy",
- "(Ljava/lang/String;J"
- "Ljava/lang/String;J)V"));
+ static jmethodID mid = 0;
+ SVN_ERR(get_editor_method(mid, "copy",
+ "(Ljava/lang/String;J"
+ "Ljava/lang/String;J)V"));
- jstring jsrc_relpath = JNIUtil::makeJString(src_relpath);
- SVN_JNI_CATCH(,SVN_ERR_RA_SVN_EDIT_ABORTED);
- jstring jdst_relpath = JNIUtil::makeJString(dst_relpath);
- SVN_JNI_CATCH(,SVN_ERR_RA_SVN_EDIT_ABORTED);
+ jstring jsrc_relpath = JNIUtil::makeJString(src_relpath);
+ SVN_JAVAHL_OLDSTYLE_EXCEPTION_CHECK(env);
+ jstring jdst_relpath = JNIUtil::makeJString(dst_relpath);
+ SVN_JAVAHL_OLDSTYLE_EXCEPTION_CHECK(env);
- SVN_JNI_CATCH(
- JNIUtil::getEnv()->CallVoidMethod(ep->m_jeditor, mid,
- jsrc_relpath, jlong(src_revision),
- jdst_relpath, jlong(replaces_rev)),
- SVN_ERR_RA_SVN_EDIT_ABORTED);
+ env.CallVoidMethod(ep->m_jeditor, mid,
+ jsrc_relpath, jlong(src_revision),
+ jdst_relpath, jlong(replaces_rev));
+ });
return SVN_NO_ERROR;
}
@@ -472,26 +495,29 @@
{
//DEBUG:fprintf(stderr, " (n) EditorProxy::cb_move('%s', r%lld, '%s')\n",
//DEBUG: src_relpath, static_cast<long long>(src_revision),
dst_relpath);
+ const ::Java::Env env;
+ SVN_JAVAHL_CATCH(env, SVN_ERR_RA_SVN_EDIT_ABORTED,
+ {
+ ::Java::LocalFrame frame(env);
- EditorProxy* const ep = static_cast<EditorProxy*>(baton);
- if (!ep || !ep->m_valid)
- return invalid_editor();
+ EditorProxy* const ep = static_cast<EditorProxy*>(baton);
+ if (!ep || !ep->m_valid)
+ return invalid_editor();
- static jmethodID mid = 0;
- SVN_ERR(get_editor_method(mid, "move",
- "(Ljava/lang/String;J"
- "Ljava/lang/String;J)V"));
+ static jmethodID mid = 0;
+ SVN_ERR(get_editor_method(mid, "move",
+ "(Ljava/lang/String;J"
+ "Ljava/lang/String;J)V"));
- jstring jsrc_relpath = JNIUtil::makeJString(src_relpath);
- SVN_JNI_CATCH(,SVN_ERR_RA_SVN_EDIT_ABORTED);
- jstring jdst_relpath = JNIUtil::makeJString(dst_relpath);
- SVN_JNI_CATCH(,SVN_ERR_RA_SVN_EDIT_ABORTED);
+ jstring jsrc_relpath = JNIUtil::makeJString(src_relpath);
+ SVN_JAVAHL_OLDSTYLE_EXCEPTION_CHECK(env);
+ jstring jdst_relpath = JNIUtil::makeJString(dst_relpath);
+ SVN_JAVAHL_OLDSTYLE_EXCEPTION_CHECK(env);
- SVN_JNI_CATCH(
- JNIUtil::getEnv()->CallVoidMethod(ep->m_jeditor, mid,
- jsrc_relpath, jlong(src_revision),
- jdst_relpath, jlong(replaces_rev)),
- SVN_ERR_RA_SVN_EDIT_ABORTED);
+ env.CallVoidMethod(ep->m_jeditor, mid,
+ jsrc_relpath, jlong(src_revision),
+ jdst_relpath, jlong(replaces_rev));
+ });
return SVN_NO_ERROR;
}
@@ -499,18 +525,21 @@
EditorProxy::cb_complete(void *baton, apr_pool_t *scratch_pool)
{
//DEBUG:fprintf(stderr, " (n) EditorProxy::cb_complete()\n");
+ const ::Java::Env env;
+ SVN_JAVAHL_CATCH(env, SVN_ERR_RA_SVN_EDIT_ABORTED,
+ {
+ ::Java::LocalFrame frame(env);
- EditorProxy* const ep = static_cast<EditorProxy*>(baton);
- if (!ep || !ep->m_valid)
- return invalid_editor();
- ep->m_valid = false;
+ EditorProxy* const ep = static_cast<EditorProxy*>(baton);
+ if (!ep || !ep->m_valid)
+ return invalid_editor();
+ ep->m_valid = false;
- static jmethodID mid = 0;
- SVN_ERR(get_editor_method(mid, "complete", "()V"));
+ static jmethodID mid = 0;
+ SVN_ERR(get_editor_method(mid, "complete", "()V"));
- SVN_JNI_CATCH(
- JNIUtil::getEnv()->CallVoidMethod(ep->m_jeditor, mid),
- SVN_ERR_RA_SVN_EDIT_ABORTED);
+ env.CallVoidMethod(ep->m_jeditor, mid);
+ });
return SVN_NO_ERROR;
}
@@ -518,17 +547,20 @@
EditorProxy::cb_abort(void *baton, apr_pool_t *scratch_pool)
{
//DEBUG:fprintf(stderr, " (n) EditorProxy::cb_abort()\n");
+ const ::Java::Env env;
+ SVN_JAVAHL_CATCH(env, SVN_ERR_RA_SVN_EDIT_ABORTED,
+ {
+ ::Java::LocalFrame frame(env);
- EditorProxy* const ep = static_cast<EditorProxy*>(baton);
- if (!ep || !ep->m_valid)
- return invalid_editor();
- ep->m_valid = false;
+ EditorProxy* const ep = static_cast<EditorProxy*>(baton);
+ if (!ep || !ep->m_valid)
+ return invalid_editor();
+ ep->m_valid = false;
- static jmethodID mid = 0;
- SVN_ERR(get_editor_method(mid, "abort", "()V"));
+ static jmethodID mid = 0;
+ SVN_ERR(get_editor_method(mid, "abort", "()V"));
- SVN_JNI_CATCH(
- JNIUtil::getEnv()->CallVoidMethod(ep->m_jeditor, mid),
- SVN_ERR_RA_SVN_EDIT_ABORTED);
+ env.CallVoidMethod(ep->m_jeditor, mid);
+ });
return SVN_NO_ERROR;
}
Index: subversion/bindings/javahl/native/NativeStream.cpp
===================================================================
--- subversion/bindings/javahl/native/NativeStream.cpp (revision 1676575)
+++ subversion/bindings/javahl/native/NativeStream.cpp (working copy)
@@ -45,16 +45,23 @@
}
NativeInputStream*
-NativeInputStream::get_self(::Java::Env env, jobject jthis)
+NativeInputStream::get_self_unsafe(::Java::Env env, jobject jthis)
{
jfieldID fid_cppaddr = NULL;
const jlong cppaddr =
findCppAddrForJObject(jthis, &fid_cppaddr, m_class_name);
- if (!cppaddr)
- ::Java::NullPointerException(env).raise(_("this [C++]"));
return reinterpret_cast<NativeInputStream*>(cppaddr);
}
+NativeInputStream*
+NativeInputStream::get_self(::Java::Env env, jobject jthis)
+{
+ NativeInputStream* self = get_self_unsafe(env, jthis);
+ if (!self)
+ ::Java::NullPointerException(env).raise(_("this [C++]"));
+ return self;
+}
+
void NativeInputStream::close(::Java::Env env, jobject jthis)
{
SVN_JAVAHL_CHECK(env, svn_stream_close(m_stream));
@@ -149,16 +156,23 @@
}
NativeOutputStream*
-NativeOutputStream::get_self(::Java::Env env, jobject jthis)
+NativeOutputStream::get_self_unsafe(::Java::Env env, jobject jthis)
{
jfieldID fid_cppaddr = NULL;
const jlong cppaddr =
findCppAddrForJObject(jthis, &fid_cppaddr, m_class_name);
- if (!cppaddr)
- ::Java::NullPointerException(env).raise(_("this [C++]"));
return reinterpret_cast<NativeOutputStream*>(cppaddr);
}
+NativeOutputStream*
+NativeOutputStream::get_self(::Java::Env env, jobject jthis)
+{
+ NativeOutputStream* self = get_self_unsafe(env, jthis);
+ if (!self)
+ ::Java::NullPointerException(env).raise(_("this [C++]"));
+ return self;
+}
+
void NativeOutputStream::close(::Java::Env env, jobject jthis)
{
SVN_JAVAHL_CHECK(env, svn_stream_close(m_stream));
@@ -294,7 +308,21 @@
return 0;
}
+JNIEXPORT void JNICALL
+Java_org_apache_subversion_javahl_types_NativeInputStream_finalize(
+ JNIEnv* jenv, jobject jthis)
+{
+ SVN_JAVAHL_JNI_TRY(NativeInputStream, finalize)
+ {
+ JavaHL::NativeInputStream* native =
+ JavaHL::NativeInputStream::get_self_unsafe(Java::Env(jenv), jthis);
+ if (native != NULL)
+ native->finalize();
+ }
+ SVN_JAVAHL_JNI_CATCH;
+}
+
// Class JavaHL::NativeOutputStream native method implementation
#include "../include/org_apache_subversion_javahl_types_NativeOutputStream.h"
@@ -337,3 +365,17 @@
}
SVN_JAVAHL_JNI_CATCH_TO_EXCEPTION(Java::IOException);
}
+
+JNIEXPORT void JNICALL
+Java_org_apache_subversion_javahl_types_NativeOutputStream_finalize(
+ JNIEnv* jenv, jobject jthis)
+{
+ SVN_JAVAHL_JNI_TRY(NativeOutputStream, finalize)
+ {
+ JavaHL::NativeOutputStream* native =
+ JavaHL::NativeOutputStream::get_self_unsafe(Java::Env(jenv), jthis);
+ if (native != NULL)
+ native->finalize();
+ }
+ SVN_JAVAHL_JNI_CATCH;
+}
Index: subversion/bindings/javahl/native/NativeStream.hpp
===================================================================
--- subversion/bindings/javahl/native/NativeStream.hpp (revision 1676575)
+++ subversion/bindings/javahl/native/NativeStream.hpp (working copy)
@@ -81,6 +81,8 @@
*/
static NativeInputStream* get_self(::Java::Env env, jobject jthis);
+ static NativeInputStream* get_self_unsafe(::Java::Env env, jobject jthis);
+
public:
/**
* Implements @c InputStream.close().
@@ -176,6 +178,8 @@
*/
static NativeOutputStream* get_self(::Java::Env env, jobject jthis);
+ static NativeOutputStream* get_self_unsafe(::Java::Env env, jobject jthis);
+
public:
/**
* Implements @c OutputStream.close().
Index:
subversion/bindings/javahl/src/org/apache/subversion/javahl/ISVNEditor.java
===================================================================
--- subversion/bindings/javahl/src/org/apache/subversion/javahl/ISVNEditor.java
(revision 1676575)
+++ subversion/bindings/javahl/src/org/apache/subversion/javahl/ISVNEditor.java
(working copy)
@@ -94,7 +94,9 @@
* <code>replacesRevision</code> set accordingly <em>must</em> be used.
* <p>
* <b>Note:</b> The <code>contents</code> stream's lifetime must not
- * extend beyond the scope of this function.
+ * extend beyond the scope of this function. An
+ * implementation <b>must</b> close the stream after
+ * consuming its contents.
*
* @throws ClientException
*/
@@ -193,7 +195,9 @@
* #addFile().
* <p>
* <b>Note:</b> The <code>contents</code> stream's lifetime must not
- * extend beyond the scope of this function.
+ * extend beyond the scope of this function. An
+ * implementation <b>must</b> close the stream after
+ * consuming its contents.
*
* @throws ClientException
*/
Index:
subversion/bindings/javahl/src/org/apache/subversion/javahl/remote/StatusEditor.java
===================================================================
---
subversion/bindings/javahl/src/org/apache/subversion/javahl/remote/StatusEditor.java
(revision 1676575)
+++
subversion/bindings/javahl/src/org/apache/subversion/javahl/remote/StatusEditor.java
(working copy)
@@ -33,6 +33,7 @@
import java.util.Locale;
import java.util.SimpleTimeZone;
import java.io.InputStream;
+import java.io.IOException;
import java.nio.charset.Charset;
/**
@@ -78,6 +79,14 @@
long replacesRevision)
{
//DEBUG:System.err.println(" [J] StatusEditor.addFile");
+ if (contents != null) {
+ try {
+ contents.close();
+ } catch (IOException ex) {
+ throw new RuntimeException(ex);
+ }
+ }
+
checkState();
receiver.addedFile(relativePath);
}
@@ -120,6 +129,14 @@
Map<String, byte[]> properties)
{
//DEBUG:System.err.println(" [J] StatusEditor.alterFile");
+ if (contents != null) {
+ try {
+ contents.close();
+ } catch (IOException ex) {
+ throw new RuntimeException(ex);
+ }
+ }
+
checkState();
receiver.modifiedFile(relativePath,
(checksum != null && contents != null),
