On 23.12.2011 23:52, Branko Čibej wrote:
On 23.12.2011 18:49, Stefan Küng wrote:
On 23.12.2011 15:18, Stefan Sperling wrote:
On Fri, Dec 23, 2011 at 04:10:07PM +0200, Daniel Shahaf wrote:
On Fri, Dec 23, 2011, at 14:50, Stefan Sperling wrote:
On Fri, Dec 23, 2011 at 02:26:40PM +0200, Daniel Shahaf wrote:
On Fri, Dec 23, 2011, at 13:01, Stefan Sperling wrote:
On Fri, Dec 23, 2011 at 01:48:18PM +0200, Daniel Shahaf wrote:
s...@apache.org wrote on Fri, Dec 23, 2011 at 00:35:16 -0000:
+ An informative error message helps people more than an
assert (which
+ in case of TSVN can mean crashing the Windows Explorer).
I thought the svn_error_malfunction_handler_t stuff addressed
that issue?
No, because TSVN didn't end up using it as intended.
See bottom of http://svn.haxx.se/users/archive-2011-10/0378.shtml
I don't see how that's relevant? That link discusses the error
dialogs.
What I'm asking is whether TSVN has implemented an
svn_error_malfunction_handler_t that replaces the default handler,
svn_error_abort_on_malfunction().
Yes it does (called svn_error_handle_malfunction()) but it calls
abort()
which means the explorer crashes:
http://tortoisesvn.googlecode.com/svn/trunk/src/SVN/SVN.cpp
And they can't remove the abort() call because...?
As Stefan explained, he cannot rely on the internal state of the
application if he gets an assertion thrown out of the svn libraries.
And he has a point. He assumes that asserts are fatal and
non-recoverable
errors. What if Subversion asserts because it just trashed the entire
stack and heap space of the windows explorer and happens to operate on
garbaga data that triggers an assertion? It doesn't make any sense to
try
to continue.
Now, we know that most of our assertions are due to bugs in our code
where, for instance, invalid data entered wc.db. But an application
using the svn libraries cannot safely rely on this knowledge.
Changing all these bogus asserts that trigger just because of invalid
working copy meta-data into proper errors is the right way to fix this.
I think I have to explain something here:
First, the shell extension of TSVN does not even call
svn_error_set_malfunction_handler() to set its own handler. Since that
function clearly states that "This function must be called in a
single-threaded context", I can not initialize that in the shell
extension since the extension can get loaded in multi threaded
situations.
And the default handler asserts and calls abort(). So the whole shell
goes down in such situations.
Then: the reason I call abort() in my own handler for TortoiseProc
(the UI part of TSVN) is that the SVN also clearly state that "If
can_return is false the method should never return. (The caller will
call abort())".
So what else than call abort() can I do? I can not return from that
function if can_return is false. No way to exit but to call abort().
So how is that "not implemented as intended"???
At least now in the UI part I can show a nasty dialog box telling the
users where to report the problem to. That way it's not me that gets
bothered with these since I can't do anything about them anyway.
I have _repeatedly_ mentioned on this list that this kind of "error
handling" (quotes intended) is bad. Sure, it won't affect the CL
client that much, but for a library that's just not acceptable.
But others have similar error handling implemented:
http://thedailywtf.com/Articles/Serious-Failure.aspx
Ranting is all very well, but I've yet to hear a suggestion from you
about how the libraries should handle unrecoverable errors. Like, for
example, the case where wc.db contains inconsistent and/or invalid data.
Simple: return an error!
That way the application can go on running, only the svn command is stopped.
Sure, returning an error isn't always easy because it requires that you
code a path to return to a known state. But for a library that's what
has to be done.
Just a real life example from a user who sent a very nasty email about
three weeks ago:
In case you didn't know, the Windows explorer is multi threaded. The
user started a backup copy operation of a 4GB directory with thousands
of files to an external drive (using explorer). Since this takes a long
time (user said about an hour) he kept working on his project while the
backup copy was in progress. About half through that copy operation, a
simple right-click on an svn working copy triggered an assertion (maybe
his working copy was corrupt?) and explorer went down.
Very nice error handling: he had to start with his backup all over again.
For me, such a behavior is not acceptable. And I've mentioned this
repeatedly here without much success.
Yes, I'm ranting. But I think I have a reason to. Because I'm pretty
sure that the asserts in the svn library will stay - you guys made it
clear every time I brought this up that you rather have the app crash
than not have a chance to get a useful stack trace from users.
I'm just wondering: how many times did you actually get a stack trace?
You're dealing with users here, not svn developers who have a debugger
installed and ready to use. And even if they have a debugger installed:
how many .NET, PHP, java, ... devs would even know how to get a stack
trace from an assert in a C app?
I think I've explained my reasons enough already, so I'll stop now.
Stefan
--
___
oo // \\ "De Chelonian Mobile"
(_,\/ \_/ \ TortoiseSVN
\ \_/_\_/> The coolest Interface to (Sub)Version Control
/_/ \_\ http://tortoisesvn.net
