You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Daniel Rall <dl...@collab.net> on 2006/10/04 01:53:22 UTC
Re: JavaHL : EXCEPTION_ACCESS_VIOLATION in checkout when destPath equals empty string
On Mon, 18 Sep 2006, Mark Phippard wrote:
> james82@gmail.com wrote on 09/18/2006 07:06:05 PM:
>
> > On 9/15/06, Alexandre Pique <ap...@isoft.fr> wrote:
> > > I have an EXCEPTION_ACCESS_VIOLATION when I call the checkout method
> of
> > SVNClientInterface with an empty destPath. I think it could be catch
> easily.
> >
> > Yes, I agree. We should definitely catch this case. If you'd like to
> > file an issue about this, please consider it buddied.
>
> I did a quick check. It looks like the code checks for NULL, but not an
> empty string.
We do check for an empty string in Path.cpp, in the code path we seem
to be passing through right before the JVM crash:
Path::init (const char * pi_path)
{
if(*pi_path == 0)
{
m_path = "";
}
...
SVNClient::checkout() creates a Path object, which in my test case --
edit BasicTest.testBasicCheckout() to pass "" for destPath -- is
producing an error with the following stack:
Native frames: (J=compiled Java code, j=interpreted, Vv=VM code, C=native code)
C [libapr-1.so.0+0x14ca7] apr_pool_destroy+0x17
C [libapr-1.so.0+0x14cb8] apr_pool_destroy+0x28
C [libsvn_subr-1.so.0+0xe8fe] svn_error_clear+0x34
C [libsvnjavahl-1.so.0.0.0+0x1665b] _ZN7JNIUtil14handleSVNErrorEP11svn_error_t+0x263
C [libsvnjavahl-1.so.0.0.0+0x25058] _ZN9SVNClient8checkoutEPKcS1_R8RevisionS3_bb+0x11a
C [libsvnjavahl-1.so.0.0.0+0x33461] Java_org_tigris_subversion_javahl_SVNClient_checkout+0x1fd
j org.tigris.subversion.javahl.SVNClient.checkout(Ljava/lang/String;Ljava/lang/String;Lorg/tigris/subversion/javahl/Revision;Lorg/tigris/subversion/javahl/Revision;ZZ)J+0
...
The crash is caused when JNIUtil::handleSVNError() calls
svn_error_clear() on an svn_error_t which has a bogus pool parameter
(the culprit), a NULL "message" field, and an unknown "apr_err" field:
...
jobject error = env->NewObject(clazz, mid, jmessage, jfile,
static_cast<jint>(err->apr_err));
printf("dying when we try to clear an svn_error_t\n");
svn_error_clear(err);
It dies when svn_error_clear() calls apr_pool_destroy(err->pool)
internally. This feels like a bug in the C++ code of the JavaHL
bindings -- can anyone see what's producing this bogus error struct?
> I did not see anywhere in the code where it checks for an
> empty string on any API. Is the checkout API the only one that would have
> this problem? Do the other SVN API's do their own checks internally and
> this one does not?
Subversion core APIs don't typically check for NULL. The 'svn'
command-line program does the check which defaults "no destination
path specified" to mean "the current directory".
> It is kind of too bad that the code is not really setup so that these sort
> of validations do not just take place in the Java layer. I guess I feel
> that way because I know Java and not really C++ so would rather make the
> change there, but there is also the unnecesary JNI calls.
>
> It looks like this change would have to take place in the C++ code though.
Re: JavaHL : EXCEPTION_ACCESS_VIOLATION in checkout when destPath equals empty string
Posted by Daniel Rall <dl...@collab.net>.
On Tue, 03 Oct 2006, Daniel Rall wrote:
> On Mon, 18 Sep 2006, Mark Phippard wrote:
>
> > james82@gmail.com wrote on 09/18/2006 07:06:05 PM:
> >
> > > On 9/15/06, Alexandre Pique <ap...@isoft.fr> wrote:
> > > > I have an EXCEPTION_ACCESS_VIOLATION when I call the checkout method
> > of
> > > SVNClientInterface with an empty destPath. I think it could be catch
> > easily.
> > >
> > > Yes, I agree. We should definitely catch this case. If you'd like to
> > > file an issue about this, please consider it buddied.
> >
> > I did a quick check. It looks like the code checks for NULL, but not an
> > empty string.
>
> We do check for an empty string in Path.cpp, in the code path we seem
> to be passing through right before the JVM crash:
>
> Path::init (const char * pi_path)
> {
> if(*pi_path == 0)
> {
> m_path = "";
> }
> ...
>
> SVNClient::checkout() creates a Path object, which in my test case --
> edit BasicTest.testBasicCheckout() to pass "" for destPath -- is
> producing an error with the following stack:
>
> Native frames: (J=compiled Java code, j=interpreted, Vv=VM code, C=native code)
> C [libapr-1.so.0+0x14ca7] apr_pool_destroy+0x17
> C [libapr-1.so.0+0x14cb8] apr_pool_destroy+0x28
> C [libsvn_subr-1.so.0+0xe8fe] svn_error_clear+0x34
> C [libsvnjavahl-1.so.0.0.0+0x1665b] _ZN7JNIUtil14handleSVNErrorEP11svn_error_t+0x263
> C [libsvnjavahl-1.so.0.0.0+0x25058] _ZN9SVNClient8checkoutEPKcS1_R8RevisionS3_bb+0x11a
> C [libsvnjavahl-1.so.0.0.0+0x33461] Java_org_tigris_subversion_javahl_SVNClient_checkout+0x1fd
> j org.tigris.subversion.javahl.SVNClient.checkout(Ljava/lang/String;Ljava/lang/String;Lorg/tigris/subversion/javahl/Revision;Lorg/tigris/subversion/javahl/Revision;ZZ)J+0
> ...
>
> The crash is caused when JNIUtil::handleSVNError() calls
> svn_error_clear() on an svn_error_t which has a bogus pool parameter
> (the culprit), a NULL "message" field, and an unknown "apr_err" field:
>
> ...
> jobject error = env->NewObject(clazz, mid, jmessage, jfile,
> static_cast<jint>(err->apr_err));
> printf("dying when we try to clear an svn_error_t\n");
> svn_error_clear(err);
>
> It dies when svn_error_clear() calls apr_pool_destroy(err->pool)
> internally. This feels like a bug in the C++ code of the JavaHL
> bindings -- can anyone see what's producing this bogus error struct?
...
This was caused by Path::m_error_occured left pointing to garbage
memory when the path is "". I fixed this in r21770 by properly
initializing m_error_occured:
Index: Path.cpp
===================================================================
--- Path.cpp (revision 21769)
+++ Path.cpp (revision 21770)
@@ -68,6 +68,7 @@
{
if(*pi_path == 0)
{
+ m_error_occured = NULL;
m_path = "";
}
else
I will propose this JVM crash fix for backport to the 1.4.x line.