[PATCH] Fix for a local reference leak in Outter.cpp

Dear Subversion Developers:

I attach a patch for a local reference leak in Outter.cpp, and the log
message follows here:

[[[
* subversion/bindings/javahl/native/Outputer.cpp
 (write): add a call to "DeleteLocalRef" where a leaking local
reference is unreachable.
]]]

To reproduce this bug, run your javaHL regression test under our
dynamic bug detector, JInn [http://userweb.cs.utexas.edu/~bclee/jinn]:

$env JAVA_TOOL_OPTIONS=-agentlib:jinn make check-javahl
...
.Exception in thread "main" xtc.lang.blink.agent.JNIAssertionFailure:
The number of local references exceeds the current capacity 16 in
NewByteArray.
        at xtc.lang.blink.agent.JNIAssertionFailure.assertFail(JNIAssertionFailure.java:16)
        at org.apache.subversion.javahl.SVNAdmin.dump(Native Method)
        at org.apache.subversion.javahl.SVNAdmin.dump(SVNAdmin.java:128)
        at org.apache.subversion.javahl.SVNTests.setUp(SVNTests.java:241)
        at junit.framework.TestCase.runBare(TestCase.java:128)
...
        at org.apache.subversion.javahl.RunTests.main(RunTests.java:116)
...

The local reference leak happens when a Java native method creates
more than 16 local references without deleting unreachable references
(DeleteLocalRef), adjusting capacity (EnsureLocalCapacity) or
allocating a local frame (PushLocalFrame). The Subversion r946780
violates this JNI programming rule:

"The JNI specification dictates that the virtual machine automatically
ensures that each native method can create at least 16 local
references. Experience shows that this provides enough capacity for
the majority of native methods that do not contain complex
interactions with objects in the Java virtual machine. If, however,
there is a need to create additional local references, a native method
may issue an EnsureLocalCapacity call to make sure that space for a
sufficient number of local references is available. For example, a
slight variation of a previous example above reserves enough capacity
for all local references created during the loop execution if
sufficient memory is available:"
[http://java.sun.com/docs/books/jni/html/refs.html#27592]

I attached gdb to the JVM, and examined every JNI call that allocates
a local reference, and I ended up the leaking allocation site at Line
99 in subversion/bindings/javahl/native/Outputer.cpp:

...
    73 svn_error_t *Outputer::write(void *baton, const char *buffer,
apr_size_t *len)
    74 {
...
    99 jbyteArray data = JNIUtil::makeJByteArray((const signed char*)buffer,
   100 *len);
   101 if (JNIUtil::isJavaExceptionThrown())
   102 return SVN_NO_ERROR;
   103
   104 // write the data
   105 jint written = env->CallIntMethod(that->m_jthis, mid, data);
   106 if (JNIUtil::isJavaExceptionThrown())
   107 return SVN_NO_ERROR;
   108
   109 // return the number of bytes written
   110 *len = written;
   111
   112 return SVN_NO_ERROR;
   113 }

This "write" method does not delete the local reference in the
variable "data" at Line 99 when it returns. In the test run, this
"write" method was activated more than 16 times. The simplest fix is
adding "env->DeleteLocalRef(data)" after Line 105 where the "data"
variable is not used in the future. Even if Line 105 throws a Java
exception, it's fine because "DeleteLocalRef" is one of 20
exception-oblivious function:

"After an exception has been raised, the native code must first clear
the exception before making other JNI calls. When there is a pending
exception, the JNI functions that are safe to call are:
....
  DeleteLocalRef()
...." [http://java.sun.com/javase/6/docs/technotes/guides/jni/spec/design.html#wp770]

For your information, here are calling contexts that allocate 16
unreachable local references:

1
Outputer::write at subversion/bindings/javahl/native/Outputer.cpp:101
svn_stream_write at subversion/libsvn_subr/stream.c:153
svn_stream_printf at subversion/libsvn_subr/stream.c:209
svn_repos_dump_fs3 at subversion/libsvn_repos/dump.c:1072
svn_repos_dump_fs2 at subversion/libsvn_repos/deprecated.c:480
SVNAdmin::dump at subversion/bindings/javahl/native/SVNAdmin.cpp:210
Java_org_apache_subversion_javahl_SVNAdmin_dump at
org_apache_subversion_javahl_SVNAdmin.cpp:165

2
Outputer::write at subversion/bindings/javahl/native/Outputer.cpp:101
svn_stream_write at subversion/libsvn_subr/stream.c:153
svn_stream_printf at subversion/libsvn_subr/stream.c:209
svn_repos_dump_fs3 at subversion/libsvn_repos/dump.c:1075
svn_repos_dump_fs2 at subversion/libsvn_repos/deprecated.c:480
SVNAdmin::dump at subversion/bindings/javahl/native/SVNAdmin.cpp:210
Java_org_apache_subversion_javahl_SVNAdmin_dump at
org_apache_subversion_javahl_SVNAdmin.cpp:165

3
Outputer::write at subversion/bindings/javahl/native/Outputer.cpp:101
svn_stream_write at subversion/libsvn_subr/stream.c:153
svn_stream_printf at subversion/libsvn_subr/stream.c:209
write_revision_record at subversion/libsvn_repos/dump.c:985
svn_repos_dump_fs3 at subversion/libsvn_repos/dump.c:1101
svn_repos_dump_fs2 at subversion/libsvn_repos/deprecated.c:480
SVNAdmin::dump at subversion/bindings/javahl/native/SVNAdmin.cpp:210
Java_org_apache_subversion_javahl_SVNAdmin_dump at
org_apache_subversion_javahl_SVNAdmin.cpp:165

4
Outputer::write at subversion/bindings/javahl/native/Outputer.cpp:101
svn_stream_write at subversion/libsvn_subr/stream.c:153
svn_stream_printf at subversion/libsvn_subr/stream.c:209
write_revision_record at subversion/libsvn_repos/dump.c:988
svn_repos_dump_fs3 at subversion/libsvn_repos/dump.c:1101
svn_repos_dump_fs2 at subversion/libsvn_repos/deprecated.c:480
SVNAdmin::dump at subversion/bindings/javahl/native/SVNAdmin.cpp:210
Java_org_apache_subversion_javahl_SVNAdmin_dump at
org_apache_subversion_javahl_SVNAdmin.cpp:165

5
Outputer::write at subversion/bindings/javahl/native/Outputer.cpp:101
svn_stream_write at subversion/libsvn_subr/stream.c:153
svn_stream_printf at subversion/libsvn_subr/stream.c:209
write_revision_record at subversion/libsvn_repos/dump.c:995
svn_repos_dump_fs3 at subversion/libsvn_repos/dump.c:1101
svn_repos_dump_fs2 at subversion/libsvn_repos/deprecated.c:480
SVNAdmin::dump at subversion/bindings/javahl/native/SVNAdmin.cpp:210
Java_org_apache_subversion_javahl_SVNAdmin_dump at
org_apache_subversion_javahl_SVNAdmin.cpp:165

6
Outputer::write at subversion/bindings/javahl/native/Outputer.cpp:101
svn_stream_write at subversion/libsvn_subr/stream.c:153
write_revision_record at subversion/libsvn_repos/dump.c:1001
svn_repos_dump_fs3 at subversion/libsvn_repos/dump.c:1101
svn_repos_dump_fs2 at subversion/libsvn_repos/deprecated.c:480
SVNAdmin::dump at subversion/bindings/javahl/native/SVNAdmin.cpp:210
Java_org_apache_subversion_javahl_SVNAdmin_dump at
org_apache_subversion_javahl_SVNAdmin.cpp:165

7
Outputer::write at subversion/bindings/javahl/native/Outputer.cpp:101
svn_stream_write at subversion/libsvn_subr/stream.c:153
write_revision_record at subversion/libsvn_repos/dump.c:1004
svn_repos_dump_fs3 at subversion/libsvn_repos/dump.c:1101
svn_repos_dump_fs2 at subversion/libsvn_repos/deprecated.c:480
SVNAdmin::dump at subversion/bindings/javahl/native/SVNAdmin.cpp:210
Java_org_apache_subversion_javahl_SVNAdmin_dump at
org_apache_subversion_javahl_SVNAdmin.cpp:165

8
Outputer::write at subversion/bindings/javahl/native/Outputer.cpp:101
svn_stream_write at subversion/libsvn_subr/stream.c:153
svn_stream_printf at subversion/libsvn_subr/stream.c:209
repos_progress_handler at subversion/libsvn_repos/deprecated.c:454
svn_repos_dump_fs3 at subversion/libsvn_repos/dump.c:1159
svn_repos_dump_fs2 at subversion/libsvn_repos/deprecated.c:480
SVNAdmin::dump at subversion/bindings/javahl/native/SVNAdmin.cpp:210
Java_org_apache_subversion_javahl_SVNAdmin_dump at
org_apache_subversion_javahl_SVNAdmin.cpp:165

9
Outputer::write at subversion/bindings/javahl/native/Outputer.cpp:101
svn_stream_write at subversion/libsvn_subr/stream.c:153
svn_stream_printf at subversion/libsvn_subr/stream.c:209
write_revision_record at subversion/libsvn_repos/dump.c:985
svn_repos_dump_fs3 at subversion/libsvn_repos/dump.c:1119
svn_repos_dump_fs2 at subversion/libsvn_repos/deprecated.c:480
SVNAdmin::dump at subversion/bindings/javahl/native/SVNAdmin.cpp:210
Java_org_apache_subversion_javahl_SVNAdmin_dump at
org_apache_subversion_javahl_SVNAdmin.cpp:165

10
Outputer::write at subversion/bindings/javahl/native/Outputer.cpp:101
svn_stream_write at subversion/libsvn_subr/stream.c:153
svn_stream_printf at subversion/libsvn_subr/stream.c:209
write_revision_record at subversion/libsvn_repos/dump.c:988
svn_repos_dump_fs3 at subversion/libsvn_repos/dump.c:1119
svn_repos_dump_fs2 at subversion/libsvn_repos/deprecated.c:480
SVNAdmin::dump at subversion/bindings/javahl/native/SVNAdmin.cpp:210
Java_org_apache_subversion_javahl_SVNAdmin_dump at
org_apache_subversion_javahl_SVNAdmin.cpp:165

11
Outputer::write at subversion/bindings/javahl/native/Outputer.cpp:101
svn_stream_write at subversion/libsvn_subr/stream.c:153
svn_stream_printf at subversion/libsvn_subr/stream.c:209
write_revision_record at subversion/libsvn_repos/dump.c:995
svn_repos_dump_fs3 at subversion/libsvn_repos/dump.c:1119
svn_repos_dump_fs2 at subversion/libsvn_repos/deprecated.c:480
SVNAdmin::dump at subversion/bindings/javahl/native/SVNAdmin.cpp:210
Java_org_apache_subversion_javahl_SVNAdmin_dump at
org_apache_subversion_javahl_SVNAdmin.cpp:165

12
Outputer::write at subversion/bindings/javahl/native/Outputer.cpp:101
svn_stream_write at subversion/libsvn_subr/stream.c:153
write_revision_record at subversion/libsvn_repos/dump.c:1001
svn_repos_dump_fs3 at subversion/libsvn_repos/dump.c:1119
svn_repos_dump_fs2 at subversion/libsvn_repos/deprecated.c:480
SVNAdmin::dump at subversion/bindings/javahl/native/SVNAdmin.cpp:210
Java_org_apache_subversion_javahl_SVNAdmin_dump at
org_apache_subversion_javahl_SVNAdmin.cpp:165

13
Outputer::write at subversion/bindings/javahl/native/Outputer.cpp:101
svn_stream_write at subversion/libsvn_subr/stream.c:153
write_revision_record at subversion/libsvn_repos/dump.c:1004
svn_repos_dump_fs3 at subversion/libsvn_repos/dump.c:1119
svn_repos_dump_fs2 at subversion/libsvn_repos/deprecated.c:480
SVNAdmin::dump at subversion/bindings/javahl/native/SVNAdmin.cpp:210
Java_org_apache_subversion_javahl_SVNAdmin_dump at
org_apache_subversion_javahl_SVNAdmin.cpp:165

14
Outputer::write at subversion/bindings/javahl/native/Outputer.cpp:101
svn_stream_write at subversion/libsvn_subr/stream.c:153
svn_stream_printf at subversion/libsvn_subr/stream.c:209
dump_node at subversion/libsvn_repos/dump.c:330
add_directory at subversion/libsvn_repos/dump.c:723
path_driver_cb_func at subversion/libsvn_repos/replay.c:450
svn_delta_path_driver at subversion/libsvn_delta/path_driver.c:254
svn_repos_replay2 at subversion/libsvn_repos/replay.c:777
svn_repos_dump_fs3 at subversion/libsvn_repos/dump.c:1152
svn_repos_dump_fs2 at subversion/libsvn_repos/deprecated.c:480
SVNAdmin::dump at subversion/bindings/javahl/native/SVNAdmin.cpp:210
Java_org_apache_subversion_javahl_SVNAdmin_dump at
org_apache_subversion_javahl_SVNAdmin.cpp:165

15
Outputer::write at subversion/bindings/javahl/native/Outputer.cpp:101
svn_stream_write at subversion/libsvn_subr/stream.c:153
svn_stream_printf at subversion/libsvn_subr/stream.c:209
dump_node at subversion/libsvn_repos/dump.c:337
add_directory at subversion/libsvn_repos/dump.c:723
path_driver_cb_func at subversion/libsvn_repos/replay.c:450
svn_delta_path_driver at subversion/libsvn_delta/path_driver.c:254
svn_repos_replay2 at subversion/libsvn_repos/replay.c:777
svn_repos_dump_fs3 at subversion/libsvn_repos/dump.c:1152
svn_repos_dump_fs2 at subversion/libsvn_repos/deprecated.c:480
SVNAdmin::dump at subversion/bindings/javahl/native/SVNAdmin.cpp:210
Java_org_apache_subversion_javahl_SVNAdmin_dump at
org_apache_subversion_javahl_SVNAdmin.cpp:165

16
Outputer::write at subversion/bindings/javahl/native/Outputer.cpp:101
svn_stream_write at subversion/libsvn_subr/stream.c:153
svn_stream_printf at subversion/libsvn_subr/stream.c:209
dump_node at subversion/libsvn_repos/dump.c:417
add_directory at subversion/libsvn_repos/dump.c:723
path_driver_cb_func at subversion/libsvn_repos/replay.c:450
svn_delta_path_driver at subversion/libsvn_delta/path_driver.c:254
svn_repos_replay2 at subversion/libsvn_repos/replay.c:777
svn_repos_dump_fs3 at subversion/libsvn_repos/dump.c:1152
svn_repos_dump_fs2 at subversion/libsvn_repos/deprecated.c:480
SVNAdmin::dump at subversion/bindings/javahl/native/SVNAdmin.cpp:210
Java_org_apache_subversion_javahl_SVNAdmin_dump at
org_apache_subversion_javahl_SVNAdmin.cpp:165

Regards,
Byeong