Re: [Plplot-devel] Error propagation

[Andrew R. <and...@us...>]

Hi all,

Sorry I have been silent for so long. Work and life have rather got in
the way of plplot development. This is something I have raised a couple
of times in the past and something I have thought about a fair bit.

Comments on Phil's proposals below.

Andrew

On Mon, 2017-01-30 at 11:00 +0000, Phil Rosenberg wrote:
> Hi all
> We have had this discussion in the past. But I would like to reopen
> it.
> 
> We really really need to get id of the exit() calls in PLplot. I
> don't
> think that we can really make any recommendation that our library is
> "industrial strength" while they are their and I would be very
> nervous
> about using PLplot in production software with them in - plus even
> using PLplot for science work I have been caught out be these calls
> in
> the past.
> 
> I think there are a few options we have got
> 
> Firstly with our internal propagation we can either do
> A1) Set up our functions so they return error codes and make sure we
> check these. I feel there is a tendency for this to be bad because I
> know that I am inherently lazy if I am frank (and I'm sure we all
> have
> our lazy moments) and tend to write code without these checks then
> (try to) go back and add them later. Missing checks are likely to
> cause segfaults or memory corruptions which are even worse than the
> exit calls().

Laziness is no excuse! The advantage of this proposal is that is allows
the library to clean things up as we unwind the stack and so (if done
properly) it will minimise any chance of problems down the line. Even
if this is a fatal error from plplot's perspective (e.g. memory
allocation failure), then we must return control to the calling
programme in a clean and controlled manner. 

> A2) Set up an error flag in PLStream which can be set on error and
> checked after every function call. This would avoid having to change
> functions that already return values, but still relies on manual
> checks which I think will be a big weak point.

I would agree that this is still subject to human laziness in checking
return codes. The advantage of explicitly returning values is that it
is at least more obvious you need to check them. On the other hand, if
functions already return values this gets tricky. We probably need to
do something of a code audit to see if this is likely to be an issue
before making final decisions.

> A3) Use setjmp/longjmp calls. In this case we call setjmp in the top
> level of every API call and on error we call longjmp which returns
> the
> execution point back to the point setjmp was called. There is some
> extra setup time here to avoid memory leaks and other resource leaks,
> but once that is done we can all write code with almost no worry
> regarding error propagation.

This has advantages in terms of not requiring explicit error code
checking. The bigger problem is making sure we tidy up properly to
avoid leaks. As Phil says, it requires some care thinking about how we
deal with non-C drivers as well. I can't say I'm too keen on
setjmp/longjmp. It can be a lazy solution to the problem rather than
writing better code in the first place, but given we have a fairly
large heritage codebase it might be an easier solution. 

> Once we decide on our internal method we need to decide how we can
> inform the user. We have a number of options again:
> 
> B1) An API change so that all our functions return an error code.

Quite a C-like approach. Requires quite a lot of changes to user code
to properly check for errors. Huge API change. Could cause problems for
the few plplot functions which already return a value. 

> B2) Make use of our current plabort call and let the user pass in a
> plabort handler.

This could get messy with some of the bindings. Calling an abort
handler also makes it hard for the programme to decide what to do. For
some uses for example you might want to shut down the plot, but
continue with the program. Harder with a handler. (I'm thinking about
things like the interactive octave bindings where I find it exceedingly
 annoying if plplot kills octave. In case of an error I might want to
return control to octave and proceed to save my work or whatever in an
orderly manner before choosing to exit or reinitialise plplot.) 

> B3) If we have an error, store an error code in PLStream and add an
> API function called plgeterr or similar which allows the user to
> check
> for error after any other function call.

Easier to implement in terms of API changes, but makes for ugly end
user code if the end user really does need to call an additional
function to check return codes every time. One plus of an internal
plstream error flag is that we could internally check it whenever a
function is called. This would prevent the user trying to continue
after an error (or just failing to check the error code). We could then
possible provide some mechanism for resetting the error flag and maybe
reinitialising plplot at the same time depending on the error? 

> B4) In the C++ binding we could throw an error. Not sure if other
> languages have similar throw/catch style options.

Other more modern languages such as java and python have exception
handling. I would very much like to see the bindings take advantage of
these where possible. It would lead to much more natural, standard and
cleaner code in these languages. 

> B5) Some combination of the above.

I'd definitely say B4 for languages that support it and B3 or B1 where
not. 

We might want to keep the option of a plabort handler (default to exit)
for now to aid compatibility in the changes. Users who are happy with
plplot exiting on error can carry on with that behaviour without having
to change their code, beyond perhaps enabling the handler. Or is that
just lazy?

> I will add there are some specific things we need to think about for
> our C++ drivers. These are not so much options, but things we need to
> deal with anyway.
> 
> C1) Our C++ drivers cannot be allowed to throw an error out of the
> driver code and into the main C code. That would potentially end up
> with the throw going all the way out to the user's top level code
> which may be in C, Fortran, or any other language that can't deal
> with
> it. Note that even if the driver doesn't call throw, it may call
> either something in the stl or another driver (in Qt or wxWidgets for
> example) that does throw. This is easy to avoid by wrapping the intry
> points in try blocks. I have done this for wxWidgets, but haven't
> checked the other C++ drivers to see if they are the same.
> 
> C2) If we go for option A3, then we cannot allow longjmp over C++
> code. This is because nontrivial destructors are not called when
> longjmp is called and if objects with these destructors are jumped
> over then the result is undefined behaviour - which again is worse
> than exit calls.
> 
> The fix for C2 - and I think this would be a good thing anyway -
> would
> be to have a specific driver API. This would allow us to have setjmp
> calls when we enter that API and avoid jumping over drivers. I think
> this would be nice in general as it is not well documented how to
> write a driver and a proper driver API would address that.
> 
> I am well aware that the use of setjmp and longjmp is very divisive.
> So my intention is to work this code up only for the case of memory
> allocation failures. This code will consist of:
> D2)A set of plmalloc, plrealloc and plfree fuctions which will
> replace
> the usual versions. As well as allocating/freeing memory these
> functions will record the allocations in the PLstream.
> D1)PLTRY, PLENDTRY macros which will go in each API call. PLTRY will
> have the setjmp call. This will be followed by the current code in
> the
> function as it is now then PLENDTRY will deal with what happens if
> longjmp has been called - it will check for memory allocations
> recorded in the PLStream and will free them. For now it will then
> just
> call exit() so we get the same behaviour but this can be changed
> later
> if we want to go down this route. Once we have this first look at how
> setjmp/longjmp can work in our code then I think it will be easier
> for
> us to make an informed choice about the A options.
> 
> Does that sound sensible to people? If anyone would like to look at
> what would be involved in doing a similar thing using the other
> options then it would be really nice to be able to compare the work
> and admin/code practices needed for those too.
> 
> Phil
> 
> -------------------------------------------------------------------
> -----------
> Check out the vibrant tech community on one of the world's most
> engaging tech sites, SlashDot.org! http://sdm.link/slashdot
> _______________________________________________
> Plplot-devel mailing list
> Plp...@li...
> https://lists.sourceforge.net/lists/listinfo/plplot-devel