Re: Octave 4.2.1 ambiguity with `clean_up_and_exit()`?

[siko1056]

John W. Eaton wrote
> On 01/11/2018 07:05 AM, siko1056 wrote:
>> jwe,
>> 
>> While looking for a solution for bug #52650 [1] I noticed, that the
>> function
>> `clean_up_and_exit()` in the standalone demo function [2] is mapped to
>> `liboctave/cruft/misc/quit.cc` [3].  I stepped with a debugger into it
>> and
>> especially the output is suspicious:
>> 
>>      GCD of [10, 15] is 5
>>      terminate called after throwing an instance of
>> 'octave::exit_exception'
>>      Aborted (core dumped)
>> 
>> The implementation [3] looks like an emergency break with the `throw`
>> statement,
>> rather than a gentle exit function.  On the other hand, there is an
>> implementation `octave::interpreter::clean_up_and_exit() in
>> `libinterp/corefcn/interpreter.cc` [4] that seems to be unused in the
>> presence
>> of [3], even within `interpreter.cc`.
>> 
>> This last aspect seems very critical to me.  So did I miss something or
>> is
>> there an ambiguity on the stable branch?  The good news is, the default
>> branch
>> seems not effected, you made a great effort in the refactoring there. 
>> The
>> bad news is, that the segmentation fault does not vanish by a simple
>> `return 0;`.
>> 
>> A similar problem I recently noticed on the mailing-list [5].  Thus what
>> is
>> the suggested method to stop the Octave interpreter in 4.2.*?  Otherwise,
>> our
>> users will have to wait until 4.4.* to get the situation fixed for
>> standalone
>> builds.
> 
> The attached version of embedded.cc should work with 4.2.
> 
> It will not work with 4.3.0+ because there is no longer an 
> embedded_application object.
> 
> As you know, for the current dev sources that will become 4.4, the 
> recommended way to create an embedded application object has changed 
> (see the new examples/code/embedded.cc file in the current sources).  I 
> think it is significantly better now.

Yes, the situation has really improved for 4.3.0+.  I mostly work with that
one and only switched back to 4.2 because of the aforementioned bug and
mailing list request.  My favorite answer in both cases was: wait for 4.4 to
become the latest official release ;-)


John W. Eaton wrote
> I don't know what the best thing is to do to provide some kind of 
> transition path.  Maybe we could restore the embedded_application object 
> (and simultaneously mark it as deprecated).  But exceptions will still 
> be an issue and existing code will probably still have to change, 
> otherwise it will likely fail if any execution_exception is thrown by 
> the embedded interpreter.
> 
> As a further transition path, we might be able to make octave_main and 
> clean_up_and_exit work again, but I'm not sure.  As it is now 
> octave_main does not work properly for embedded interpreters because it 
> creates a temporary embedded_application object that is deleted when 
> octave_main returns.  That could be changed to a global or static object 
> so that it would still exist after octave_main returns, but there are 
> still issues with catching the quit exception.

According to the number of complains, on the bug tracker there are four
since 4.2:

  #52650 - Segfault when loading package via C++ API
  #50974 - SEGFAULT with constructed cmdline_options
  #50111 - In embedded mode Octave parser not always reports parse error
  #50068 - Segfault, when eval_string is used in interpreter embedded mode

it is not a major problem for using Octave.  Oct-files work fine.

Bug #50068 is very interesting, as exactly one year ago, the same
observations were made, but no real solution was found against the
termination segfault.  You fixed that bug for the default branch (#50068
comment #17), but the situation on stable was left unchanged and probably
forgotten (because marked as fixed).

My major assertion was, that any call of `clean_up_and_exit()` gets mapped
to `liboctave/cruft/misc/quit.cc`, rather than
`octave::interpreter::clean_up_and_exit()` which is now removed on default.

For me to continue working on the last bug #52650 I need to know, whether
any call of `clean_up_and_exit()` has nothing to do with the version from
`liboctave/cruft/misc/quit.cc`.

Even on default a grepping for `clean_up_and_exit()` reveals, that
`liboctave/cruft/misc/quit.cc:clean_up_and_exit()` is used nowhere in
Octave.  Maybe a candidate for removal? ;-)


John W. Eaton wrote
> As I recall, the quit exception is thrown instead of just exiting so 
> that Octave can exit cleanly from the top level.  What happens with the 
> current stable sources if clean_up_and_exit just calls exit?  Does it 
> really clean up?  With the current dev sources, it might work OK because 
> more interpreter resources are cleaned up by class destructors.
> 
> When I revised this for the current dev sources, I could not see a way 
> to preserve the old interface and keep existing code working.  Sorry.
> 
> Do you have any ideas about how to fix it while maintaining binary 
> compatibility with previous 4.2.x releases?  I couldn't see a way, but 
> that doesn't mean there isn't something that could be done.
> 
> jwe

I will try to remove `liboctave/cruft/misc/quit.cc:clean_up_and_exit()` and
see what happens for stable,

Kai



--
Sent from: http://octave.1599824.n4.nabble.com/Octave-Maintainers-f1638794.html