Re: Re-enterable related exception in SVNKit since rev5872

> Hey, no problem.  The data I'm backing up with this represents about
> 5-6 years of some really hard work on my part.  I wouldn't be able to do it
> without SVNKit (and some of the commercial products that use SVNKit) so I'm
> happy to help when I can.
> As an update my little hack fix does appear to be working fine.
> I've successfully synced about 11000 revisions so far and it's running
> happily, so thank you.
> As for SVN, should I report the bug, or would you prefer to?  I
> think I understood the whole test case, so I could probably script that and
> report it easily enough if you haven't already.
>
> -Greg
>
>> -----Original Message-----
>> From: Alexander Sinyushkin [mailto:Alexander.Sinyushkin@...]
>> Sent: Friday, June 05, 2009 5:06 AM
>> To: svnkit-users@...
>> Subject: Re: Re-enterable related exception in SVNKit since rev5872
>>
>> Hello Greg,
>>
>> Nice catch :) Actually you've found a bug not only in SVNKit, but in
>> Subversion as well. I tried a little test with syncing a little test
>> repository where an svn:eol-style property was set on a file and then
>> deleted, and the native svnsync crashed with a segmentation fault. I'm
>> not sure if the Subversion community is already aware of this bug,
>> maybe
>> it's worth writing them about this.
>>
>> Concerning your notes of SVNRepository.lock() method: SVNDebugLog just
>> logs a stack trace to a log file to provide information on where the
>> race condition problem occurred. That's ok.
>>
>> It does make sence to me as well, that our try-catch blocks should
>> include handling Runtime exceptions and converting them to
>> SVNException.
>> We would probably discuss that afterwards.
>>
>> The fact that an svn:keywords property is passed as null just means
>> that it was deleted in some certain revision. Just like svn:eol-style
>> in
>> my test.
>>
>> Finally, I have committed the fix in trunk. Again, thank you for your
>> big help :)
>> ----
>> Alexander Sinyushkin,
>> TMate Software,
>> http://svnkit.com/ - Java [Sub]Versioning Library!
>>
>> Greg Gibeling wrote:
>>> I built the JARs from rev5872 and then 5888 (HEAD) of trunk, and
>>> when I tried to use them to re-synchronize my gigantic repositories
>> (the
>>> target of the sync is empty and recently created with the same
>> version of
>>> SVNKit) I got the following exception:
>>>
>>> java.lang.Error: SVNRepository methods are not reenterable
>>> at
>>> org.tmatesoft.svn.core.io.SVNRepository.lock(SVNRepository.java:2818)
>>> at
>>> org.tmatesoft.svn.core.io.SVNRepository.lock(SVNRepository.java:2809)
>>> at
>>>
>> org.tmatesoft.svn.core.internal.io.fs.FSRepository.openRepositoryRoot(F
>> SRepo
>>> sitory.java:748)
>>> at
>>>
>> org.tmatesoft.svn.core.internal.io.fs.FSRepository.openRepository(FSRep
>> osito
>>> ry.java:739)
>>> at
>>>
>> org.tmatesoft.svn.core.internal.io.fs.FSRepository.setRevisionPropertyV
>> alue(
>>> FSRepository.java:149)
>>> at
>>>
>> org.tmatesoft.svn.core.internal.io.fs.FSRepository.setRevisionPropertyV
>> alue(
>>> FSRepository.java:143)
>>> at
>>>
>> org.tmatesoft.svn.core.internal.io.fs.FSRepository.setRevisionPropertyV
>> alue(
>>> FSRepository.java:139)
>>> at
>>>
>> org.tmatesoft.svn.core.wc.admin.SVNAdminClient.unlock(SVNAdminClient.ja
>> va:15
>>> 59)
>>> at
>>>
>> org.tmatesoft.svn.core.wc.admin.SVNAdminClient.doSynchronize(SVNAdminCl
>> ient.
>>> java:702)
>>>
>>> There are three issues here:
>>> 1) The throw site (SVNRepository.java:2818) seems to be doing
>>> something very odd.  It logs a exception with an empty message and
>> then
>>> throws an error.  That could be intended, but it's a little strange.
>>>
>>> 2) The exception/error handling in SVNAdminClient does not handle
>>> Errors and RuntimeExceptions very well.  In particular,
>> doSynchronize() has:
>>> } catch (SVNException svne) {
>>> error = svne;
>>> } finally {
>>>
>>> However if there's a RuntimeException or Error thrown inside the
>>> try{} block, it will miss the catch and the finally will be invoked.
>> But
>>> because the finally{} throws its own Error (the re-enterable related
>> message
>>> above) this first Error/RuntimeException from the try{} is completely
>> lost.
>>> I did this to get the real error out:
>>>
>>> } catch (SVNException svne) {
>>> error = svne;
>>> } catch (Error temp) {
>>> error = new
>>> SVNException(SVNErrorMessage.create(SVNErrorCode.UNKNOWN, temp),
>> temp);
>>> } catch (RuntimeException temp) {
>>> error = new
>>> SVNException(SVNErrorMessage.create(SVNErrorCode.UNKNOWN, temp),
>> temp);
>>> } finally {
>>>
>>> A similar change to the exception catch for error2 in the
>> finally{}
>>> block is advisable.  As is a similar change to other methods with
>> this
>>> structure which many of the doXXX methods in the admin client have.
>>>
>>> 3) Once I got that all sorted out the exception from the try{}
>> block
>>> turned out to be the following NPE:
>>>
>>> Caused by: java.lang.NullPointerException
>>> at
>>>
>> org.tmatesoft.svn.core.wc.admin.SVNAdminClient.normalizeString(SVNAdmin
>> Clien
>>> t.java:1606)
>>> at
>>>
>> org.tmatesoft.svn.core.internal.wc.SVNSynchronizeEditor.changeFilePrope
>> rty(S
>>> VNSynchronizeEditor.java:105)
>>> at
>>>
>> org.tmatesoft.svn.core.internal.wc.SVNCancellableEditor.changeFilePrope
>> rty(S
>>> VNCancellableEditor.java:135)
>>> at
>>>
>> org.tmatesoft.svn.core.internal.io.svn.SVNEditModeReader.processCommand
>> (SVNE
>>> ditModeReader.java:155)
>>> at
>>>
>> org.tmatesoft.svn.core.internal.io.svn.SVNEditModeReader.driveEditor(SV
>> NEdit
>>> ModeReader.java:214)
>>> at
>>>
>> org.tmatesoft.svn.core.internal.io.svn.SVNRepositoryImpl.replay(SVNRepo
>> sitor
>>> yImpl.java:852)
>>> at
>>>
>> org.tmatesoft.svn.core.io.SVNRepository.replayRange(SVNRepository.java:
>> 2292)
>>> at
>>>
>> org.tmatesoft.svn.core.wc.admin.SVNAdminClient.doSynchronize(SVNAdminCl
>> ient.
>>> java:703)
>>>
>>> Note that the line numbers may be a little off (I added a little
>>> temporary debug code).  That's at the first line of normalizeString()
>>> causing an NPE because the argument "string" is null.
>>> The solution I found, at least for now, is to change that first
>> line
>>> from:
>>>
>>> if (string.indexOf(SVNProperty.EOL_CR_BYTES[0]) != -1) {
>>>
>>> to
>>>
>>> if ((string != null) && (string.indexOf(SVNProperty.EOL_CR_BYTES[0])
>> != -1))
>>> {
>>>
>>> Since the default return value is null anyway, this seemed
>> entirely
>>> reasonable.  This definitely allows significantly more revisions to
>> be
>>> synchronized.
>>> In case you're wondering it's an "svn:keywords" property which
>>> results in the null value, and no I don't know why its null, that
>> does seem
>>> a little odd to me.
>>>
>>> -Greg
>>>
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: svnkit-users-unsubscribe@...
>>> For additional commands, e-mail: svnkit-users-help@...
>>>
>>>
>>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: svnkit-users-unsubscribe@...
>> For additional commands, e-mail: svnkit-users-help@...
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: svnkit-users-unsubscribe@...
> For additional commands, e-mail: svnkit-users-help@...
>
>
>