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

2010-08-31 Thread Daniel Shahaf
Johan Corveleyn wrote on Tue, Aug 31, 2010 at 22:07:59 +0200:
 (I hope the attachments (file svn-crash...log.txt and svn-crash...dmp) get
 through; if not, let me know and I'll zip them or something)

I see svn-crash-log20100831213059.log.txt only.

By the way, please send as plain text, if possible. (you sent
multipart/alternative with an html part)


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

2010-08-30 Thread Bert Huijben


 -Original Message-
 From: Daniel Shahaf [mailto:d...@daniel.shahaf.name]
 Sent: zondag 29 augustus 2010 16:36
 To: Johan Corveleyn
 Cc: dev@subversion.apache.org; stef...@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,  stef...@apache.org wrote:
   Author: stefan2
   Date: Sun Aug 29 10:32:08 2010
   New Revision: 990537
  
   URL: http://svn.apache.org/viewvc?rev=990537view=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-bra...@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

2010-08-30 Thread Stefan Fuhrmann

Bert Huijben wrote:
  

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

Author: stefan2
Date: Sun Aug 29 10:32:08 2010
New Revision: 990537

URL: http://svn.apache.org/viewvc?rev=990537view=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-bra...@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

2010-08-29 Thread Johan Corveleyn
On Sun, Aug 29, 2010 at 4:36 PM, Daniel Shahaf d...@daniel.shahaf.namewrote:

 Johan Corveleyn wrote on Sun, Aug 29, 2010 at 16:24:24 +0200:
  Hi Stefan,
 
  On Sun, Aug 29, 2010 at 12:32 PM,  stef...@apache.org wrote:
   Author: stefan2
   Date: Sun Aug 29 10:32:08 2010
   New Revision: 990537
  
   URL: http://svn.apache.org/viewvc?rev=990537view=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-bra...@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