subversion-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Byeongcheol Lee <lineonk...@gmail.com>
Subject [PATCH] Fix for Invalid Local References
Date Fri, 21 May 2010 18:59:03 GMT
Dear Subversion Developers:

I attach a patch for the invalid local references in the
"CopySources.cpp", and the log message is here:

[[[
* subversion/bindings/javahl/native/CopySources.cpp
(array): Replace "JNIStringHolder" with two calls to
"GetStringUTFChars" and "DeleteLocalRef".
]]]

To produce 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 JNI reference 0x6857e68c
is dead in the argument 2 of ReleaseStringUTFChars.
	at xtc.lang.blink.agent.JNIAssertionFailure.assertFail(JNIAssertionFailure.java:16)
	at org.apache.subversion.javahl.SVNClient.copy(Native Method)
	at org.apache.subversion.javahl.BasicTests.testCopy(BasicTests.java:913)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
	at java.lang.reflect.Method.invoke(Method.java:597)
	at junit.framework.TestCase.runTest(TestCase.java:164)
	at junit.framework.TestCase.runBare(TestCase.java:130)
	at junit.framework.TestResult$1.protect(TestResult.java:106)
	at junit.framework.TestResult.runProtected(TestResult.java:124)
	at junit.framework.TestResult.run(TestResult.java:109)
	at junit.framework.TestCase.run(TestCase.java:120)
	at junit.framework.TestSuite.runTest(TestSuite.java:230)
	at junit.framework.TestSuite.run(TestSuite.java:225)
	at junit.framework.TestSuite.runTest(TestSuite.java:230)
	at junit.framework.TestSuite.run(TestSuite.java:225)
	at junit.textui.TestRunner.doRun(TestRunner.java:121)
	at junit.textui.TestRunner.doRun(TestRunner.java:114)
	at junit.textui.TestRunner.run(TestRunner.java:77)
	at org.apache.subversion.javahl.RunTests.main(RunTests.java:116)
...

The relevant source lines are here.

$ cat -n  subversion/bindings/javahl/native/CopySources.cpp
...
    85	apr_array_header_t *
    86	CopySources::array(SVN::Pool &pool)
    87	{
...
    99	  for (std::vector<jobject>::const_iterator it = sources.begin();
   100	        it < sources.end(); ++it)
   101	    {
...
   119	      JNIStringHolder path(jpath);
...
   126	      env->DeleteLocalRef(jpath);
...
   170	    }
...
   175	}

The "array" method allocates, and frees and uses a local reference at
Line 119, 126, and 99. The use at Line 99 is implicit. because C++
destructor of "JNIStringHolder" runs before the second iteration of
the for loop from Line 99 to 170. The source lines and calling context
at the pointer of failure  are here.

$cat -n ./subversion/bindings/javahl/native/JNIStringHolder.cpp
...
    46	JNIStringHolder::~JNIStringHolder()
    47	{
    48	  if (m_jtext && m_str)
    49	    m_env->ReleaseStringUTFChars(m_jtext, m_str);
    50	}


~JNIStringHolder at subversion/bindings/javahl/native/JNIStringHolder.cpp:49
CopySources::array  at subversion/bindings/javahl/native/CopySources.cpp:99
SVNClient::copyat subversion/bindings/javahl/native/SVNClient.cpp:438
Java_org_apache_subversion_javahl_SVNClient_copyat
subversion/bindings/javahl/native/org_apache_subversion_javahl_SVNClient.cpp:588

I looked at the definition and uses of "JNIStringHolder",  and It's
better not to use the "JNIStringHolder" within a loop. My patch
replaces "JNIStringHolder" with a few calls to JNI functions.

Regards,
Byeong

Mime
View raw message