Re: FYI: unwind_protect improved

[John W. Eaton]

On 24-Jun-2009, Jaroslav Hajek wrote:

| Oh, so that's where it came from... now it makes more sense.

Another thing to keep in mind when you are looking at these parts
of Octave is that C++ did not have exception handling when I started
working on Octave.

| To be
| honest, I don't particularly like the global unwind_stack idea - it
| doesn't play nicely with user handling using try-catch and
| (especially) try-finally, which I think is more C++-ish.

Try-finally?  Is that standard C++?

| The problem
| is that when interrupt or bad_alloc occurs, the whole unwind_protect
| stack is run (via hook), then an exception is raised which unwinds the
| (system) call stack. If a user tries to handle something ussing
| try/finally, in the finally block the unwind_protect stack will
| already be processed, and the user must be aware of it - but it seems
| counter-intuitive to me.
| OTOH, we can't empty the stack after the exception is propagated to
| the top level, because the cleanup handlers can (and do, in some
| places) refer to local variables.

Where are there references to local variables in the unwind_protect
stack?  I don't doubt that some have slipped in, but it was never
really intended for that.

| In a certain sense, the unwind_protect mechanism is more powerful
| (allows dynamic adding of handlers), but does Octave really need it?
| Wouldn't error handling based purely on try/catch/finally be
| sufficient? Maybe faster, too. I think gcc has gone far with exception
| handling optimization. What do you think?

I don't object to improvements, or even a complete reimplementation,
but let's discuss it in detail here first.

| FYI, I pushed in some more improvements concerning unwind_protect,
| atexit, quit, Ctrl-C and the like. It seems octave_quit_exception was
| not that good idea after all, as it does not propagate through
| liboctave Fortran callbacks, and cleaning unwind_stack has the
| problems described above. For now, I've removed it. I also
| transplanted the patch to 3.2.x. octave_quit_exception was not marked
| as API class, and probably not used by any C++ code, so I don't see
| this as a problem.

OK.

| The following three programs (also attached) will now do what you
| think they should (fail in 3.2.0):
| 
| tt1.m:
| 
| function xdot = ttt (x, t)
|   xdot = 1;
|   unwind_protect
|     if t > 0.5
|       t
|       quit();
|     endif
|   unwind_protect_cleanup
|     disp ("hi");
|   end_unwind_protect
| endfunction
| 
| unwind_protect
|   a = lsode (@ttt, 1, 0:.1:1);
| unwind_protect_cleanup
|   disp ("hihi");
| end_unwind_protect
| 
| tt2.m:
| 
| atexit ("quit")
| quit()
| 
| tt3.m:
| 
| function dumb
|   fprintf (stderr, "press Ctrl-C:\n")
|   unwind_protect
|     sleep (10)
|     quit ()
|   unwind_protect_cleanup
|     disp ("out!")
|   end_unwind_protect
| endfunction
| atexit("dumb")
| unwind_protect
|   quit ()
| unwind_protect_cleanup
|   fprintf (stderr, "me too!\n")
| end_unwind_protect

Should we add these (or something like them) as tests?

| > I think the names "saved_var" and "saved_mem" might be better than
| > "restore_var" and "restore_mem".  It usually seems better to me to
| > name objects as things rather than actions.
| 
| I wanted to the word "restore" in the name to emphasize that it will
| restore the value on cleanup, and "restorator" or something seemed bad
| to me. I don't think saved_var quite hits the meaning, but if you
| think so, feel free to change it.

What about var_to_restore and mem_to_restore?  But that seems about
the same to me as saved_var and saved_mem.

OK.

jwe