Re: FYI: unwind_protect improved

[Jaroslav Hajek]

On Wed, Jun 24, 2009 at 2:20 PM, John W. Eaton<address@hidden> wrote:
> 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.

Yeah, I know. And a lot of liboctave code is from pre-templates era.
Surprisingly, some of the macro code is actually more readable than an
equivalent template code would be.

> | 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++?

No, of course not, I'm being stupid. That's Java. OK, given the
absence of try/finally in C++, unwind_protect makes much more sense.

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

I think they're calls to symbol_table::set_scope with a pointer to
local saved scope. To avoid these, it may be a good idea to provide a
mechanism in unwind_protect to pack a function pointer together with
an argument. I'll try to create something.
Afterwards, we can remove the hooks and simply unwind the stack when
an exception propagates to the lower level, which will ensure that a
function's frame exists until it is explicitly unwinded.

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

No, I take back the reimplementation, that was a result of the
try/finally nonsense. But I'd like to remove the instant unwinding at
interrupts or bad_allocs.

> | 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?

Probably. However, I don't think our current mechanism supports tests
where quitting the interpreter is included.
>
> | > 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.

I'm not sure. The idea of verbs=functions, nouns=classes has its
merits. Maybe var_protector and mem_protector?

-- 
RNDr. Jaroslav Hajek
computing expert & GNU Octave developer
Aeronautical Research and Test Institute (VZLU)
Prague, Czech Republic
url: www.highegg.matfyz.cz