On 02.05.2019 12:16, Johan Corveleyn wrote:
> On Thu, May 2, 2019 at 10:34 AM Johan Corveleyn <jcorvel_at_gmail.com> wrote:
>> On Tue, Apr 30, 2019 at 9:28 PM Eliot, Christopher
>> <christopher.eliot_at_nagrastar.com> wrote:
>>> I am not subscribed to this mailing list. If you need to reach me, please CC me explicitly at christopher.eliot_at_nagrastar.com
>> Hi Christopher,
>>
>>> Running TortoiseSVN1.10 (r28148).
>>>
>>> Accessing SVN repository as https://ndev-svn01/svn/...
>>>
>>> TortoiseSVN had been running for a long time, I had done lots of things in it.
>>> I right-clicked on a tag and selected "Mark for comparison"
>>> I clicked on the trunk.
>>> Can't recall if I clicked "compare URLs" or not.
>>>
>>> Ooh. This is repeatable. I shut down TortoiseSVN complete and restarted it.
>>> Performed the same sequence of events; I get this same exception when I click on
>>> "compare URLs". I reproduced it a couple of times.
>>>
>>> I then selected the trunk and marked it for comparison, selected the tag, and
>>> was able to to compare them just fine.
>>>
>>> I then went back and repeated the original sequence of events and again got the
>>> exception.
>> Thanks for taking the time to report this.
>>
>> However, I'm afraid this community (the "core" subversion project)
>> can't help you directly. I think it's likely that this problem is
>> situated somewhere in the TortoiseSVN code (which is a separate
>> project, for the Windows GUI on top of the Subversion libraries). It
>> seems it is internally calling the Subversion libraries with a
>> "non-canonical path". Maybe there is something strange with the urls
>> that are constructed here?
>>
>> If you report this to the TortoiseSVN community
>> (https://tortoisesvn.net/community.html), they might be able to
>> provide more help or to ask more specific questions to find out what's
>> going on.
>>
>>> I'll look into updating TortoisSVN.
>>>
>>> When I update to 1.12, it’s unusable. When I click on the directory for my project, I get a popup that just says “TortoisesSVN client has stopped working. A problem caused the program to stop working correctly. Please close the program.” It seems to be a problem with just this project; I can click down into other projects in the same repository. I’ll report that separately.
>> Here again I think you should probably contact the TSVN community. It
>> might be related to the first problem (some non-canonical url being
>> passed to the libraries, specific to that one project). Or it might be
>> something else.
>>
>>> ---------------------------
>>> Subversion Exception!
>>> ---------------------------
>>> Subversion encountered a serious problem.
>>> Please take the time to report this on the Subversion mailing list
>>> with as much information as possible about what
>>> you were trying to do.
>>> But please first search the mailing list archives for the error message
>>> to avoid reporting the same problem repeatedly.
>>> You can find the mailing list archives at
>>> https://subversion.apache.org/mailing-lists.html
>>>
>>> Subversion reported the following
>>> (you can copy the content of this dialog
>>> to the clipboard using Ctrl-C):
>>>
>>> In file
>>> 'D:\Development\SVN\Releases\TortoiseSVN-1.10.0\ext\subversion\subversion\libsvn_ra\ra_loader.c'
>>> line 629: assertion failed (svn_relpath_is_canonical(path))
>>> ---------------------------
>>> OK
>>>
>>> Topher Eliot
> Hi devs,
>
> I'm afraid I've done it again: replied to a user report of an
> "svn_relpath_is_canonical" assertion (crashing TortoiseSVN) by asking
> the OP to report it to TSVN. Maybe this is not ideal (sorry Stefan),
> but I don't know what else to do with reports like that. So after a
> couple of days of non-response I try to help by giving at least "some
> option to move things along".
>
> Can we do better? I know this has been asked before (most recently
> last December [1]), but we stumbled somewhere along the way as far as
> I can see. I know this is a sensitive area prone to "finger-pointing",
> but IMHO we should keep on trying :-).
>
> Some things that came out of that discussion last December:
>
> 1) Brane committed new canonicalization functions in r1848943 [2].
> What's the status of those? Are they being used by TSVN now, and
> perhaps part of TSVN 1.12 (thus explaining the different crash that OP
> observed with TSVN 1.12)?
>
> Also: Julian ran some "random input tests" on those new functions [3],
> bringing a couple of things to light. Not sure whether those things
> were all addressed. AFAICS Julian's tests were not committed.
All I can say is that there is no way for our canonicalisation functions
to produce the correct result for *every* possible input. The new
variants that return errors instead of aborting are an improvement, but
they won't magically fix the underlying problem. At least now, clients
can report an error to the user.
> 2) I asked about adding the actual path on which the assert is failing
> to the error message, to help troubleshooting. Julian sent a patch for
> discussion [4], but concerns were raised about users inadvertently
> leaking sensitive info with their error reports. It seems that
> discussion got a bit stuck, and nothing was committed.
We do report failed paths elsewhere in our error messages. Adding them
to this particular error message would be no worse than what we're
already doing. Adding them to a crash report might be worse.
> 3) The bigger issue was also raised [5]: "We should not be doing abort
> from a library". That discussion got stuck too, AFAICS. Maybe it
> should be revisited (again) ...
Over the years, we've done quite a bit of work towards eliminating
random aborts from the library. However, not all such call sequences
have been eliminated; there's still room for improvement. The "problem"
is that any change we make here may require one or more API revisions
just to be able to return an error.
The one case where, I think, we don't want to "improve" is handling
failed allocations (from APR pools etc.), because graceful OOM handling
from a library is very, very hard to get right. Since our library
doesn't create the pools and allocators (at least, clients *should* be
doing that), clients already have the option to use APR's callback for
handling allocation failures. (Yes, we have svn_pool_allocator_create(),
but we don't force clients to use it.)
> Realistically, I won't hold my breath for point (3), as long as we're
> on 1.x. But I would be overjoyed if we could make further progress
> with (1) and (2).
(3) is just a lot of code churn and doesn't require a 2.x release.
Anyone with some time and patience on their hands can work towards that
goal, it's not hard, just nitpicky detail work.
-- Brane
Received on 2019-05-03 11:26:06 CEST