[GRASS-dev] Vect_*_fatal_error()

[Sören Gebbert]

Hi Martin,

2011/11/17 Martin Landa <landa.martin at gmail.com>:
> Hi,
>
> 2011/11/12 Glynn Clements <glynn at gclements.plus.com>:
>> Even if the exit()-on-fatal-error issue magically disappeared, the
>> memory management issues (i.e. don't bother tracking allocations which
>> don't matter for a typical module) won't. Nor will the fact that
>> "try ... except" won't catch a segfault.
>>
>> grass.lib.* exists for "module-style" scripts, not for persistent
>> applications.
>
> that's right, but we just shouldn't forbid to use GRASS libraries for
> such applications (for which the GRASS libraries were not designed).

This is not forbidden ... but it is a very bad idea to do so.

> As we all know they were designed for the GRASS modules. I just think
> that it's very very bad political decision to *forbid* the programmer
> to changes behaviour of G_fatal_error(), or better to say to avoid
> calling exit() if he/she really wants to go on even if there was a
> fatal error. It's not over business, we just say, that the GRASS
> libraries has been designed for that cases, so you will remain in
> undefined stage and you should force the user to quit the whole
> application or so. The reason why I raised this issue again I think
> that giving no chance to change behaviour of G_fatal_error() is just
> ignoring and making life very difficult to non-GRASS or currently
> wxGUI programmers. I really think that we should give the programmer
> possibility to override G_fatal_error() by something which is not
> calling exit() if he/she really needed and let him/her know that the
> GRASS libraries remains in undefined stage. It's not over business,
> it's up to the programmer.

You already can override G_fatal_error() to avoid exit() by using
G_set/add_error_routine() using longjmp and setjmp. The
vtk-grass-bridge uses this feature to catch G_fatal_error() exit()
calls. Due to the undefined state of the library after calling
G_fatal_error(), this is only useful to prepare meaningful error
messages and pass them to the persistent application to inform the
user to exit the persistent application or to restart it.

IMHO the main wxGUI should not make use of library functions which
call G_fata_error() directly, it should use grass modules or start
Python processes which call grass library functions. These processes
can make use of Glynns Python longjmp approach to pass meaningful
error messages to the wxGUI before they get restarted by the wxGUI.

>>> Well due to design, anyway very user
>>> friendly. I am not a fan of G_set_fatal_error() solution, on the other
>>> hand I don't see any reason why to *forbid* so strictly the
>>> programmers which are using GRASS libraries to avoid calling system
>>> exit on G_fatal_error(). The default behaviour will remain! We just
>>> let other programmers (with a big warning) to use GRASS libraries for
>>> the purpose which they were not originally designed nothing more.
>>
>> Allowing G_fatal_error() to return isn't going to stop wxGUI, QGIS,
>> etc terminating unexpectedly. It's just going to make them terminate
>> via a segfault rather than via exit().
>
> Not always in every case. What about Rast_open_old() which calls also

But in most cases.

> G_fatal_error(). If you try in that application to open raster map
> (which fails) the application just crashes without no message. Good;-)
> You have no possibility to show message what failed why it failed and
> eg. force the user to quit the application if needed.

You have.

>
>> If a function calls G_fatal_error(), it isn't expecting it to return.
>> If it does return, the usual result will be that the caller attempts
>> to use invalid data; a crash isn't likely to be far off.
>>
>> A typical example of G_fatal_error() usage is:
>>
>>        FILE *fp = fopen(...);
>>        if (!fp)
>>            G_fatal_error(...);
>>        fread(..., fp);
>>
>> Allowing G_fatal_error() to return will simply replace the exit() with
>> a segfault. Why would this be an improvement?
>
> The improvement is that you can show at least some error message and
> to quit application with knowing user that the fatal error appeared,
> where appeared, etc. In the most cases it will be bug in the
> application, at least the application will just not crash without any
> message.
>
>> If you want to (attempt to) recover from fatal errors, you have to
>> escape from G_fatal_error() via a longjmp(). Either that, or you have
>> to re-write every single use of G_fatal_error() (~3800 of them) to
>> accomodate the possibility of it returning.
>
> I would prefer to modify G_fatal_error() rather then use longjmp()
> which is just basically a workaround. Moreover the application can
> still fail in the same way. It's not a safe solution.

Using longjmp() is not a work around, its IMHO the only meaningful solution.
I would like to cite Glynn again:
" If you want to (attempt to) recover from fatal errors, you have to
escape from G_fatal_error() via a longjmp(). Either that, or you have
to re-write every single use of G_fatal_error() (~3800 of them) to
accomodate the possibility of it returning."

>
>> Except that the latter isn't really an acceptable solution, as it
>> pushes the burden of error handling on to everything else. You might
>> consider making your job easier justifies making everyone else's
>> harder, but don't expect everyone else to agree.
>>
>> So long as I have commit privileges, G_fatal_error() is going to
>> retain (and honour) the "noreturn" attribute.
>
> WHY SO? That are very very strong words to me! I thought that the
> GRASS project is still community based and nobody has a veto
> privilege. Probably I am wrong. Please consider also my reasoning.

These are indeed strong words. But i also think its a very bad idea to
change this design decision.
I was starting the discussion some time ago to prepare GRASS libraries
for persistent applications.
I learned and have changed my mind. Because the fundamental design of
GRASS and therefor all implemented modules depend on the exit() on
fatal error design decision. It will be almost impossible to change
this if you don't want to rewrite most of the code.

A solution is to use processes and communicate between them with
serialized objects. Thats "easy" in Python and Java but quite hard in
C/C++.

Consider the main wxGUI as the master process, able to start, stop and
restart subprocesses. These subprocesses are doing specific tasks
(vector editing, raster cell editing, ...) by accessing the GRASS
library directly and using longjmp() to report to the master process
in case of a G_fatal_error() call.

Best regards
Soeren