Re: Re-enterable related exception in SVNKit since rev5872

by Alexander Sinyushkin :: Rate this Message:

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(FSRepo
> sitory.java:748)
> at
> org.tmatesoft.svn.core.internal.io.fs.FSRepository.openRepository(FSReposito
> ry.java:739)
> at
> org.tmatesoft.svn.core.internal.io.fs.FSRepository.setRevisionPropertyValue(
> FSRepository.java:149)
> at
> org.tmatesoft.svn.core.internal.io.fs.FSRepository.setRevisionPropertyValue(
> FSRepository.java:143)
> at
> org.tmatesoft.svn.core.internal.io.fs.FSRepository.setRevisionPropertyValue(
> FSRepository.java:139)
> at
> org.tmatesoft.svn.core.wc.admin.SVNAdminClient.unlock(SVNAdminClient.java:15
> 59)
> at
> org.tmatesoft.svn.core.wc.admin.SVNAdminClient.doSynchronize(SVNAdminClient.
> 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(SVNAdminClien
> t.java:1606)
> at
> org.tmatesoft.svn.core.internal.wc.SVNSynchronizeEditor.changeFileProperty(S
> VNSynchronizeEditor.java:105)
> at
> org.tmatesoft.svn.core.internal.wc.SVNCancellableEditor.changeFileProperty(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(SVNEdit
> ModeReader.java:214)
> at
> org.tmatesoft.svn.core.internal.io.svn.SVNRepositoryImpl.replay(SVNRepositor
> yImpl.java:852)
> at
> org.tmatesoft.svn.core.io.SVNRepository.replayRange(SVNRepository.java:2292)
> at
> org.tmatesoft.svn.core.wc.admin.SVNAdminClient.doSynchronize(SVNAdminClient.
> 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@...

« Return to Thread: Re-enterable related exception in SVNKit since rev5872