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:
>>>>>>>> stsp_at_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
Received on 2011-12-24 09:42:14 CET