[GRASS-dev] Re: problem with G_fatal_error

[Glynn Clements]

Martin Dobias wrote:

> > > > The problem looks like this: QGIS calls Vect_open_old_head function -
> > > > that one finds out that the layer is corrupted and calls G_fatal_error
> > > > routine. Looking at that function I see that in 6.3.cvs the function
> > > > always calls exit() which is incorrect, because this takes also the
> > > > whole QGIS down. GRASS 6.2 has a condition "if ( ext_error ) return
> > > > 0;" before the exit statement, so since QGIS sets extended error
> > > > handler, everything is okay. Please could you add that condition back?
> >
> > No.
> >
> > Code which calls G_fatal_error() is entitled to assume (and normally
> > does assume) that G_fatal_error() will not return.
> >
> > If you don't want G_fatal_error() to terminate the process, you need
> > to ensure that your fatal error handler doesn't return, but e.g.
> > longjmp()s back to your code.
> 
> Hi Glynn,
> 
> don't you think it's very inconvenient for users of GRASS libraries
> that stable API changes its semantics?

Its intended semantics haven't changed; it was a bug which has been
fixed.

It was only noticed when __attribute__((noreturn)) was added to the
function and gcc pointed out that it might actually return.

Note that the GRASS 6 Programmer's Manual has said the following since
at least 2004/02/20 (the point that it was moved into CVS):

	<P>
	int G_set_error_routine(int (*handler)()) change error handling
	
	...
	
	<P> <B>Note.</B> The handler only provides a way to send the message
	somewhere other than to the error output. If the error is fatal, the
	module will exit after the handler returns.

> And isn't that weird that a
> library function calls exit() to close the process it is using it?

Not when the normal use of the function is to terminate the process.

It needs to be borne in mind that the GRASS libraries were (and are)
designed for GRASS, which is a collection of "one-shot" programs (i.e. 
you run the program with arguments, it does its work, then exits). The
design of the libraries is fundamentally unsuitable for persistent
programs.

If you feel like re-writing the whole of GRASS to propagate all errors
up to the individual programs and handle them there (and cleaning up
any inconsistent state along the way), I'm sure that the effort will
be appreciated.

> Btw. thinking about such incompatible changes, is there any resource
> on internet or low-traffic mailing list which would announce any such
> changes? Because this is not the first time that something got
> changed... :-/

The only resource which documents every bug-fix is the grass-commit
list, and that certainly isn't low-traffic. Even if we had an
-announce list, there's no reason to suspect that this particular
change would have been announced there; it isn't as if anyone would
have known that you were relying upon this particular detail.

> I'll take a look at longjmp()

Don't forget this part from my previous message:

> You will need to bear in mind that any variables used by the GRASS
> libraries will be in an undefined state after this point, so the
> effect of calling any GRASS function is undefined.

longjmp() will "save" QGIS, but once something calls G_fatal_error(),
the GRASS libraries should be considered toast. Further calls to GRASS
functions may well result in a segfault, silent corruption of data,
etc.

-- 
Glynn Clements <glynn at gclements.plus.com>