On 2011-12-23 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 answer:
Return an normal error, giving the caller (e.g. TSVN shell extension)
the chance for example to continue its work on other working copies.
But definitely never just abort or assert (crash the Windows explorer).
This is standard in today's applications.
For example, when opening a corrupt document in OpenOffice,
in the worst case you may get an error "corrupt file",
but this does not crash OpenOffice itself,
but the user can choose to open another file.
So there must be never "unrecoverable errors".
There may be some unrecoverable data (e.g. a broken db/WC),
but the caller must be able to continue work on other data
(e.g. another WC).
Cheers,
Folkr
Received on 2011-12-24 09:47:17 CET