Re: [OctDev] [PATCH] CVS-octave-forge and GCC3.2

[John W. E. <jw...@be...>]

On 27-Nov-2002, Paul Kienzle <pki...@ja...> wrote:

| I've sprinkled very ugly OCTAVE_QUIT and BEGIN/END_INTERRUPT_IN_FOREIGN_CODE
| macros throughout the sparse functions.
| 
| sp_test continues to work, even after Ctrl-C.  But if you have top running,
| you can see that Ctrl-C leads to big memory leaks.

This would have been true with the old method of handling interrupts
too, only it would have been even worse because it just jumped back to
the top level, ignoring all resource allocations on the way back to
the top.  Only those that were explicitly added to Octave's
unwind_protect stack would have been deallocated/restored, and in most
cases, Octave's unwind_protect stack is used not to protect resource
allocations, but to ensure that temporary modifications to global
variables are restored.

Now you can control where OCTAVE_QUIT is called.  The resource
allocation problem can be solved by ensuring that resources are
allocated in constructors and deallocated in destructors.  If that's
not possible, then another approach would be to do something like

  if (octave_interrupt_state)
    {
      // Deallocate resources/restore state/etc here.

      octave_interrupt_state = 0;
      octave_throw_interrupt_exception ();
    }

instead of using OCTAVE_QUIT (which is the same as the code above,
except it doesn't explicitly deallocate or restore any resources).

| There is a new debug_on_interrupt flag.  I have confirmed that it will
| drop you into a debug prompt on Ctrl-C, but I haven't checked that it
| is in a valid state when you do.  In particular, I don't know if the
| current line has been run to completion, or if dbg_cont will cause it
| to be executed again.  The few simple tests I did (sp_test followed
| by Ctrl-C followed by dbg_cont) seem to work.

I think it should be OK, because when debug_on_interrupt is set, the
SIGINT handler just does

  octave_debug_on_interrupt_state = true;

and returns.  Unless you send another interrupt before entering the
debugger, octave_interrupt_state will not be set, and Octave will
continue executing until the statement evaluation loop is hit, where
we check the value of octave_debug_on_interrupt_state.  At that point,
we are preparing to evaluate the next statement, so everything
should be in a sane state.

| I don't know how to generate internal sparse errors, so I don't know that
| I have SuperLU compiled correctly to throw fatal errors through C
| code.

For testing, I suppose you could just modify the SuperLU code to
generate one of these errors for a valid call, or modify the way
SuperLU is called so that the call will generate an error.  We have
the same problem with testing what happens when Lapack code is called
with incorrect arguments.  Normally there is not supposed to be any
way to make that happen from Octave, so to test it you have to
introduce an error in code, then remove it after testing is done.

| I'm hoping that these errors will all be memory errors so that throwing a 
| bad_alloc exception is appropriate.

OK.  I haven't really looked at SuperLU, so I don't know, but if there
are other possible errors, it would be nice to distinguish between bad
allocations and other problems.

| Elsewhere sparse is using SP_FATAL_ERROR rather than laboriously 
| returning the error up the stack.  E.g., sparse(1,1,[1,2])
| Since it is SP_FATAL_ERROR which is throwing the bad_alloc exception,
| this is erroneously labeled a "memory exhausted" error.  I'm willing
| to live with that for the moment, but I'm happy to accept patches to
| fix it properly.

OK.

| I don't want to put to much energy into it until I know which sparse
| package John is going to use in Octave.  I've seen reports on the web that
| umfpack and mumps are both faster than superlu, so maybe we will change
| packages.  I know in particular that umfpack does propogates error codes
| rather than punting to a fatal error handler, which will remove at least
| some of the problems in the code.

I will try to respond later today to your other message about sparse
packages that we might be able to use.

Thanks,

jwe