Bert Huijben wrote:
>
>> -----Original Message-----
>> From: Daniel Shahaf [mailto:d.s_at_daniel.shahaf.name]
>> Sent: zondag 29 augustus 2010 16:36
>> To: Johan Corveleyn
>> Cc: dev_at_subversion.apache.org; stefan2_at_apache.org
>> Subject: Re: svn commit: r990537 -
>> /subversion/branches/performance/subversion/libsvn_subr/svn_temp_ser
>> ializer.c
>>
>> Johan Corveleyn wrote on Sun, Aug 29, 2010 at 16:24:24 +0200:
>>
>>> Hi Stefan,
>>>
>>> On Sun, Aug 29, 2010 at 12:32 PM, <stefan2_at_apache.org> wrote:
>>>
>>>> Author: stefan2
>>>> Date: Sun Aug 29 10:32:08 2010
>>>> New Revision: 990537
>>>>
>>>> URL: http://svn.apache.org/viewvc?rev=990537&view=rev
>>>> Log:
>>>> Looking for the cause of Johan Corveleyn's crash (see
>>>> http://svn.haxx.se/dev/archive-2010-08/0652.shtml), it
>>>> seems that wrong / corrupted data contains backward
>>>> pointers, i.e. negative offsets. That cannot happen if
>>>> everything works as intended.
>>>>
>>> I've just retried my test after this change (actually with
>>> performance-branch_at_990579, so updated just 10 minutes ago). Now I get
>>> the assertion error, after running log or blame on that particular
>>> file:
>>>
>>> [[[
>>> $ svnserve -d -r c:/research/svn/experiment/repos
>>> Assertion failed: *ptr > buffer, file
>>> ..\..\..\subversion\libsvn_subr\svn_temp_serializer.c, line 282
>>>
>>> This application has requested the Runtime to terminate it in an unusual
>>>
>> way.
>>
>>> Please contact the application's support team for more information.
>>> ]]]
>>>
>>> Is there any way I can find more information about this failure, so I
>>> can help you diagnose the problem?
>>>
>>>
>> s/assert/SVN_ERR_MALFUNCTION_NO_RETURN/
>>
>
> assert and SVN_ERR_MALFUNCTION() have their own use.
>
> assert() is only evaluated in Maintainer/DEBUG mode and isn't evaluated at
> runtime for normal users. So this assertion is good for tests that are too
> slow for general use or for things that shouldn't happen but are not really
> an error.
> E.g. it is used through svn_dirent_*(), svn_uri_*(), etc.
> [A url is really just a valid unix path which can be canonicalized by
> removing double slashes, but using it is in almost every case a user error.
> So this triggers a maintainer/debug only assertion]
>
> This allows easier debugging of these errors.
>
> SVN_ERR_MALFUNCTION() and SVN_ERR_ASSERT() are for tests that also need
> evaluation in release mode. The fatal error is avoidable by installing a
> error callback.
>
>
> The SVN_ERR_MALFUNCTION_NO_RETURN and SVN_ERR_ASSERT_NO_RETURN() function
> should be avoided (real fix: Make the function return svn_error_t*) as these
> will trigger an error and then an abort at runtime. So it will CRASH third
> party application when this error occurs there.
>
>
I'm a bit torn what the ultimate solution would be here. For the time
being,
I only need that as debug code. Depending on the root cause of the problem,
I might "promote" it to "general threat" and then use one of the NO_RETURN
errors because it is still better than a PF.
But it would never be a good choice to just continue operations (e.g.
disable
the cache and retry) because the server may already operate on or even have
handed out on corrupted data.
-- Stefan^2.
Received on 2010-08-30 12:22:40 CEST