Re: JavaHL testBasicLocking() crash

On Tue, 10 Apr 2007, Mark Phippard wrote:

> On 4/9/07, Paul Burba <pburba@collab.net> wrote:
> >When running these JavaHL tests from trunk on Windows:
> >
> >org.tigris.subversion.javahl.tests.BasicTests.testCopy()
> >org.tigris.subversion.javahl.tests.BasicTests.testBasicLocking()
> >org.tigris.subversion.javahl.tests.BasicTests.testMergeInfoRetrieval()
> >
> >I get errors like this:
> >
> >C:\SVN\src-trunk>java -cp
> >C:\SVN\src-trunk\subversion\bindings\javahl\classes;"C:\Program
> >Files\Java\jdk1.6.0\jre\lib\ext\junit.jar" org.tigris.subversion.
> >javahl.tests.RunTests
> >..........E...#
> ># An unexpected error has been detected by Java Runtime Environment:
> >#
> ># EXCEPTION_ACCESS_VIOLATION (0xc0000005) at pc=0x10003b50, pid=4612,
> >tid=4024
> >#
> ># Java VM: Java HotSpot(TM) Client VM (1.6.0-b105 mixed mode)
> ># Problematic frame:
> ># C [libsvnjavahl-1.dll+0x3b50]
> >#
> ># An error report file with more information is saved as
> >hs_err_pid4612.log
> >#
> ># If you would like to submit a bug report, please visit:
> ># http://java.sun.com/webapps/bugreport/crash.jsp
> >#
> >
> >Looking at the lock test failure in detail I think I found the problem,
> >but I'm a bit mystified as to why it it's not a problem on other
> >platforms.
> >
> >First, the problem:
> >
> >eh's r23949 introduced the svn_wc__entry_versioned() macro which called
> >the new svn_wc__entry_versioned_internal() wrapper around
> >svn_wc_entry(). When SVN_DEBUG is not defined the macro called
> >svn_wc__entry_versioned_internal() with the const char *caller_filename
> >arg set to NULL, this looks ok because svn_error_t's docs say we don't
> >use in when not debugging:
> >
> > /** Source file where the error originated. Only used iff @c
> >SVN_DEBUG. */
> > const char *file;
> >
> >
> >The problem arises when r23949 meets dlr's r24263 in
> >subversion/bindings/java/javahl/native/JNIUtil.cpp:
> >
> > void JNIUtil::handleSVNError(svn_error_t *err)
> > {
> > std::string msg;
> > assembleErrorMessage(err, 0, APR_SUCCESS, msg);
> >+ std::ostringstream source;
> >+ source << err->file << ':' << err->line;
> > throwNativeException(JAVA_PACKAGE "/ClientException", msg.c_str(),
> >- err->file, err->apr_err);
> >+ source.str().c_str(), err->apr_err);
> > svn_error_clear(err);
> > }
> >
> >When we try to insert NULL (i.e. err->file) into the std::ostringstream
> >stream is when I get the EXCEPTION_ACCESS_VIOLATION mentioned at the
> >start. It's been a long time since I've written any C++, but my
> >understanding is that you can't insert NULL like this but need to use
> >ostream& ends ( ostream& os )?
> >
> >The fix for this problem seems really simple:
> >
> >[[[
> >Avoid EXCEPTION_ACCESS_VIOLATION error when running JavaHL tests on
> >Win32.
> >
> >* subversion/bindings/javahl/native/JNIUtil.cpp
> > (handleSVNError): Don't insert null pointer into std::ostringstream.
> >]]]
> >
> >[[[
> >Index: subversion/bindings/javahl/native/JNIUtil.cpp
> >===================================================================
> >--- subversion/bindings/javahl/native/JNIUtil.cpp (revision 24504)
> >+++ subversion/bindings/javahl/native/JNIUtil.cpp (working copy)
> >@@ -363,7 +363,7 @@
> > std::string msg;
> > assembleErrorMessage(err, 0, APR_SUCCESS, msg);
> > std::ostringstream source;
> >- source << err->file << ':' << err->line;
> >+ source << (err->file ? err->file : "") << ':' << err->line;
> > throwNativeException(JAVA_PACKAGE "/ClientException", msg.c_str(),
> > source.str().c_str(), err->apr_err);
> > svn_error_clear(err);
> >]]]
> >
> >But I have no idea why this only affects Win32 and that makes me mildly
> >suspicious that something else is going on. Any thoughts from those
> >with more C++ and/or Java knowledge?
> >
>
> Does anyone have the time to look at this? Currently the JavaHL
> bindings crash a lot on Windows, apparently due to this issue.

I committed a similar patch to trunk in r24517. Thanks guys!