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. -- Brane
