You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Johan Corveleyn <jc...@gmail.com> on 2010/08/29 14:24:24 UTC

Re: svn commit: r990537 - /subversion/branches/performance/subversion/libsvn_subr/svn_temp_serializer.c

Hi Stefan,

On Sun, Aug 29, 2010 at 12:32 PM,  <st...@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@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?

Just to be clear: the very same repos does not have this problem when
accessed by a trunk svnserve.

-- 
Johan

Re: svn commit: r990537 - /subversion/branches/performance/subversion/libsvn_subr/svn_temp_ser ializer.c

Posted by Stefan Fuhrmann <st...@alice-dsl.de>.
Bert Huijben wrote:
>   
>> -----Original Message-----
>> From: Daniel Shahaf [mailto:d.s@daniel.shahaf.name]
>> Sent: zondag 29 augustus 2010 16:36
>> To: Johan Corveleyn
>> Cc: dev@subversion.apache.org; stefan2@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,  <st...@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@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.

Re: svn commit: r990537 - /subversion/branches/performance/subversion/libsvn_subr/svn_temp_ser ializer.c

Posted by Stefan Fuhrmann <st...@alice-dsl.de>.
Bert Huijben wrote:
>   
>> -----Original Message-----
>> From: Daniel Shahaf [mailto:d.s@daniel.shahaf.name]
>> Sent: zondag 29 augustus 2010 16:36
>> To: Johan Corveleyn
>> Cc: dev@subversion.apache.org; stefan2@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,  <st...@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@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.


RE: svn commit: r990537 - /subversion/branches/performance/subversion/libsvn_subr/svn_temp_ser ializer.c

Posted by Bert Huijben <be...@qqmail.nl>.

> -----Original Message-----
> From: Daniel Shahaf [mailto:d.s@daniel.shahaf.name]
> Sent: zondag 29 augustus 2010 16:36
> To: Johan Corveleyn
> Cc: dev@subversion.apache.org; stefan2@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,  <st...@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@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.

	Bert

Re: svn commit: r990537 - /subversion/branches/performance/subversion/libsvn_subr/svn_temp_ser ializer.c

Posted by Johan Corveleyn <jc...@gmail.com>.
On Sun, Aug 29, 2010 at 4:36 PM, Daniel Shahaf <d....@daniel.shahaf.name>wrote:

> Johan Corveleyn wrote on Sun, Aug 29, 2010 at 16:24:24 +0200:
> > Hi Stefan,
> >
> > On Sun, Aug 29, 2010 at 12:32 PM,  <st...@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@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/
>


I think you meant SVN_ERR_ASSERT_NO_RETURN.

If I do that, I get:

[[[
$ svnserve -d -r c:/research/svn/experiment/repos
..\..\..\subversion\libsvn_fs_fs\fs_fs.c:2363: (apr_err=235000)
svn: In file '..\..\..\subversion\libsvn_subr\svn_temp_serializer.c' line
282: assertion failed (*ptr > buffer)

This application has requested the Runtime to terminate it in an unusual
way.
Please contact the application's support team for more information.

This application has requested the Runtime to terminate it in an unusual
way.
Please contact the application's support team for more information.
]]]


-- 
Johan

Re: svn commit: r990537 - /subversion/branches/performance/subversion/libsvn_subr/svn_temp_ser ializer.c

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Johan Corveleyn wrote on Sun, Aug 29, 2010 at 16:24:24 +0200:
> Hi Stefan,
> 
> On Sun, Aug 29, 2010 at 12:32 PM,  <st...@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@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/

> Just to be clear: the very same repos does not have this problem when
> accessed by a trunk svnserve.
> 
> -- 
> Johan