Still an issue in Octave 6.x (bug #58454).  Tested example of comment #0 with HG ID https://hg.savannah.gnu.org/hgweb/octave/rev/2b52e473b6ef (stable).

The patch from comment #44 doesn't apply cleanly anymore,
"Hunk #1 FAILED at 91"
The code for column sorting has changed too much I'm afraid, much more than I can deal with (my C++ proficiency is a bit meager).

Could you (when you have time) please update this patch?

Thanks

I got hit by the recent changes in the build system and ran out of time.
Maybe maybe I can have another try but we're off on vacation very soon. If not it'll be mid-August before I can report back.

Sorry for the late reply, I'm trying to finish other urgent or long-pending stuff before my vacation and I got little time ATM anyway.

Does your patch from comment #44 need to be applied after that of https://savannah.gnu.org/bugs/index.php?45366 ?

You've characterized things pretty well now, thanks.  I think if we can just figure out how to cleanly kill the thread so that Windows doesn't complain, then we are close.  The other issue with qt-settings could maybe be separate.  The time to wait seems like a question.  Maybe rather than wait 2 seconds, it could be changed to 4 or 5 seconds.  Here's the code hunk that was applied and should exist in the file main_window.cc:

+void
+main_window::force_exit_app (bool closenow)
+{
+  if (! closenow)
+    return;
+
+  // If confirmed, do not allow shutdown sequence again.  Doing
+  // so would mean something is wrong with the core's exit
+  // method and could lead to indefinite loop.
+  _closing = true;
+
+  // First interrupt will stop a process, so attempt exit.
+  _octave_qt_link->terminal_interrupt ();
+  octave_cmd_exec *cmd = new octave_cmd_exec ("exit");
+  queue_command (cmd);
+  _exit_in_queue = true;
+
+  // Give the core/worker two seconds to wrap up.
+  QTimer *timer = new QTimer ();
+  connect (timer, SIGNAL (timeout ()), this, SLOT (terminate_and_exit ()));
+  timer->setSingleShot (true);
+  timer->start (2000);
+}

Notice at the very end of the routine is 2000, the number of milliseconds.  If you want to experiment, you could change that to 10000 seconds and see how prevalent the popup warning is.

I too thought I had seen the corrupted qt-settings file upon testing.  But I didn't think anything of it because I was in the middle of debugging.  The only way I can think of that happening is if a qSettings object is writing the settings at the instant a process is ended.  But that should live in the GUI space, which supposedly is not the thread being killed.  Anyway, this one I can probably think through and figure out what the issue is.

The performance test is interesting, i.e., the fact that Windows is so much faster, but the two changesets affect code that is only exercised at exit.  I can't imagine that is taking very much longer.  BTW, I sort of prefer the performance test

start_time = cputime();
<process>
overall_time = cputime() - start_time()

The tic/toc are clock time and some aspects of Octave have a delay but really aren't using CPU.

Well, back to the exit problems on Windows.  At first things looked good; guess that's progress.  You'll have to characterize the behavior a little more and maybe it will give some ideas.  Try some things like:

1) Correlate the zombie occurrence with the Windows popup message.  Are there any times when there is a zombie and no message?  What is the Windows message exactly?

2) See if there are any problems with a task that doesn't use so much memory.  Try "pause(20)".  Maybe for the eigenvalues example the task of either interrupting or exiting takes so long to clean up memory that the allotted two second wait expires and the GUI kills the process and that is when a Windows popup appears.  It could be that this cleanup takes a random amount of time, sometimes finishing, sometimes not.

If the latter situation arises, we may need to figure out how to cleanly kill a process in Windows.  Perhaps that isn't being done exactly right.

Performance degradation?  What test are you running that measures performance?  The only thing that should be affected is clean up and exit behavior--any processing should be unaffected.  Also, whatever you are measuring shows large enough deviation that the sample size you have is probably inconclusive.

Anyway, I'm looking forward to seeing if Windows works.  Hope so!  :-)

Thanks, I'll try later this week  (I just upgraded my laptop with an SSD and have to re-install a lot of SW)

The changeset for https://savannah.gnu.org/bugs/index.php?45366 comes before attempting the forced-exit.  We can wait for that changeset to be reviewed/applied, which is never a certainty.  Maybe later this weekend I will apply that changeset locally and build another changeset to attach here.  (The two changesets would need to be applied in order.)

Sure I will, but I have several other priorities the next days. I'll try to squeeze it somewhere in anyway :-)

Thanks for your perseverance with this code cleanup. You've surely turned a lot of stones along the way.

Well, the Windows behavior certainly isn't acceptable.  After some digging, I think I see why this is happening.  The QSettings is instantiated and deleted by the worker thread.  Why that is, I don't know.  Doesn't seem very object oriented to have something in another thread handling the settings for what is in another thread.  I suppose the 1/2 second wait might not be long enough for the worker to finish cleanup and the thread happens to be deleted right in the middle of deleting that QSettings object.  (This can probably happen on Linux too, it just might be Windows is a little slower cleaning up.)  I'll bring this up on the discussion list.

The other issue is Windows displaying a message saying that Octave quit unexpectedly.  (Another indication that the worker was in fact deleted rather than finishing in the conventional way.)  It would be nice to figure out how to prevent Windows from displaying such a message in that circumstance in the cases where Octave is forcibly quit because it might be stuck.

Here's a patch that I think is very near to what I've envisioned the closing process should be like.  It is a considerable rearrangement of the closing code which relies more on the connections of signals and slots.  The connections sort of act like a linked-list or serial processing of editor tabs.  One tab's save-continue signal is connected to the next tab's save-confirm slot.  If there is a cancellation, the tab doesn't emit the signal.

The proposed setup has more flexibility to shape the behavior of dialog boxes when closing and preventing things like a dialog opening multiple times.  So if there are any changes to be made, only small tweaks should be necessary.

Also, I did come across the long, verbose error message that I think you are seeing.  I'm not 100% certain, but I sort of isolated the cause as being related to the Qt Handles code.  If the interpreter thread is terminated, the message originates after the termination.  I think there is some kind of coordination going on between the interpreter and Qt Handles that is compromised when the interpreter quits prematurely.  I'd rather not dig into the Qt Handles code--at least not initially.  Judging from bug reports, there are certain parts of Qt use that aren't as robust as others and I suspect there will be changes somewhere in the future.  Nonetheless, the new scheme is set up so that terminating the interpreter is the last resort and the error message shouldn't appear so often if at all.

Here's a summary of changes:

1) Selecting "quit" or the upper-right close button will place an "exit" in the queue, as before.

2) A timer expires after 2 seconds if no shutdown is requested by the core and a dialog box is shown inquiring about "force exit", as before.

3) The existence of signal/slot connections determines if a dialog box is already present so that only one "force exit" dialog exists at a time.  (I.e., can no longer select "quit" twice quickly to create two dialog boxes asking to force quit.)

4) If the user selects force quit, the confirm-save process is started.  If that confirm-save process is complete before the interpreter asks to shutdown, the force-exit route is taken.

5) The force-exit route will issue a Cntrl-C, place "exit" in the queue again, and wait 0.5 seconds for a response.  Typically, in this scenario the interpreter will ask to shutdown at this point and another confirm-save will NOT be done.  Also, this is a clean exit so no error messages should appear.

6) If 5 still doesn't exit, that's an indication of the core really stuck/lost.  After half a second, the 3 Cntrl-C approach is attempted followed by terminating the interpreter.  This is a sort of last effort type of thing.  Here's when the error message might occur and if someone files a bug report for this I will look at the Qt Handles code.

7) And one last detail, if the user does a force-exit and is in the process of saving editor tabs when/if the interpreter gets around to requesting shutdown, the signal of completion is disconnected and reconnected instead to a slot that is the conventional response.  That makes sure that save-confirm isn't run twice simultaneously.


(file #34183)

Does this output appear anywhere:

panic: Interrupt -- stopping myself...

in the case of Windows?  My guess is not.  The three control-Cs probably aren't aborting the interpreter, so the change is effectively no different than previous and the crash message happens upon GUI exit.

I'll have to dig a little deeper.  Either find some graceful route for the interpreter to exit, or modify the exit signal message (from the GUI thread, so a semaphore is likely needed) so that the screen dump isn't done.  Tried the former quite a bit, so probably the latter will yield a solution.

I'm checking here occasionally and still have this feature in the back of my mind, but I've very limited on time at the moment.

Here's a new patch with a new approach.  Rather than attempt to make QThread exit gracefully and/or try to change the Octave interpreter's signal handling (something I really don't want to change, no less before a release), let's try to use the interpreter's existing mechanism for aborting the program: Cntrl-C three times in a row.

In the attached patch I've done a forced exit by issuing four Cntrl-Cs in a row.  The first one won't contribute to "three-in-a-row" if it properly interrupts out of a process.  So, using the forced exit, I'm hoping the worst you see is the same as what is seen at the command line:

octave:1> inv(rand(10000))
^C^CPress Control-C again to abort.
^Cpanic: Interrupt -- stopping myself...
attempting to save variables to 'octave-workspace'...
save to 'octave-workspace' complete

I also changed the wait time to 2.0 seconds rather than 3.0.

I'm OK with the Cntrl-C abort approach, but I'm still not 100% sure of the flow is just right.  The one issue I see right now is if during forced exit and confirmation on open files the original "exit" gets processed and one "Cancel"s the exit via file-save, another immediate file-save confirmation will be attempted.

I will think about "double confirm" issue one a bit, but in mean time, please try this patch and see if it works better for you.

(file #33994)

Rather than a backtrace, the output would be "octave exited with value 6", meaning the program didn't exit in the conventional manner.  Guess I will aim for this behavior.

One can see the list of signals by typing "SIG" at the octave command line and can probably guess what many of the signals mean.  ABRT (6) means to abort the program.  INT (2) means interrupt.  There are even real archaic signals like HUP (1) which I think means "hang up"--computer comm links long ago were made by dialing into the system then placing the telephone receiver into a holder that would modulate tones to send data (at a really low baud rate).  To disconnect, hang up the telephone.

How about a non-zero exit notification, i.e., SIGABRT?  Should we try for that?

When disappeared without a trace, was that when there were no modified files in the editor?

Also, is it possible that the backtrace appeared in the GUI terminal window which disappeared so quickly it's barely noticeable?  When the GUI is launched, at first the OS terminal window receives the standard-error output, but then the GUI takes hold of that.  On shutdown, the GUI gives the stderr output back to the terminal window.  There could be a race condition between that output redirection and the Octave core worker processes aborting.  In one case the GUI might close before worker process displays a message, in another case after.

I think we should get a consensus on the expected behavior.  I'm alright with a backtrace output.  The shutdown is forced, after all, and the force operation is achieving what is wanted.  I'm a little reluctant to design it to issue an interrupt and then attempt exit again after the interrupt, because if the problem with the core isn't solved with an interrupt (i.e., it is truly stuck, as opposed to merely busy), then that strategy becomes and indefinite loop.

(the verbatim copy in comment #16 is only a few lines; the start of the backtrace scrolled out of the Konsole terminal)

I was working on a related bug (#44751) which I wanted to make sure was working first...

OK, in the new attached patch I've added the "check for modified" confirmation when forcing the quit.  I've also interrupted the interpreter before quitting the interpreter to see if that makes a difference for you.

After this I think I will open this to the discussion list to see if others can think of a cleaner way of shutting down the interpreter thread, but it would be good to know how this works for you.

(file #33743)

Oh no worries, I get sidetracked all the time :)

I will try your new patch this weeekend.

Thanks

Good question.  That error looks like it is coming from the Octave core, right?  I assumed that quitting the application would gracefully stop the Octave CLI thread.  But apparently not.  I'm going to think about this one for a bit, and if you'd like to try something, let me know if it works.  It should be just one or two strategically placed signals or function calls.

The question I have is whether we might have to stop the CLI thread (actually two threads that I see).  The reason is that if we issue the current Qt signal that causes and interrupt, i.e.,

  connect (command_window, SIGNAL (interrupt_signal (void)),
           _octave_qt_link, SLOT (terminal_interrupt (void)));

and

void
terminal_dock_widget::terminal_interrupt (void)
{
  emit interrupt_signal ();
}

it's doesn't solve this issue (I think) because signals across threads are queued, slots across threads aren't processed immediately, unlike in-thread signals/slots.  I think we'd have to simply stop the threads.

As I said, there are two threads, one for the octave CLI and one for the command queue:

      QApplication application (argc, argv);

      octave_cli_thread main_thread (argc, argv);

      application.setQuitOnLastWindowClosed (false);

      main_thread.start ();

      return application.exec ();

and

octave_qt_link::octave_qt_link (QWidget *p)
  : octave_link (), main_thread (new QThread ()),
    command_interpreter (new octave_interpreter ())
{
  connect (this, SIGNAL (execute_interpreter_signal (void)),
           command_interpreter, SLOT (execute (void)));

  connect (command_interpreter, SIGNAL (octave_ready_signal ()),
           p, SLOT (handle_octave_ready ()));

  command_interpreter->moveToThread (main_thread);

  main_thread->start ();

We need some way of accessing and stopping these threads directly, not with slots.  There need to be a couple extra pointers and short functions to do that.

I'll think about for a day or so.

Give the attached patch a try.  I think it is the best that can be done right now.

Upon running Octave GUI, edit a file so that the GUI asks to save the file upon selecting File|Exit or Cntrl-Q.  Try all various options like

[select File|Exit]

pause(10)
[select File|Exit]
[wait longer than 10 seconds, the force quit dialog should close automatically because core finished]
[select "Cancel" for the save file dialog]

pause(20)
[select File|Exit]
[select "Cancel" button when dialog appears]
[select File|Exit]
[select "Cancel" button when dialog appears]
[select File|Exit]
[select "Cancel" button when dialog appears]
[select "Cancel" for the save file dialog]

pause(20)
[select File|Exit]
[select "Force Exit" button when Force Exit dialog appears]
[all gone]

(file #33305)

Agree with both of you, a confirmation popup would be a good idea, similar to when there are unsaved files open in the editor.

I'm not sure about the delayed timer-based idea, wouldn't it be better to just detect whether Octave is busy at the time the user tries to exit and show the dialog then?

Another thing is that AFAIU the main menu's File | Exit instruction sequence is similar or equal to what the OS sends to Octave when e.g., shutting down.
Indeed, when I enter:

for ii=1:1000
eigs (randn (400))
endfor

...and try to shutdown the system I get a popup that program "Octave" is unresponsive.

In far-fetched cases (say, laptop battery nearly exhausted) having Octave obstruct system actions is a bad idea; although I expect any OS to be able to simply ~"kill -9" Octave.
So while I agree with Dan about consistent paths I think the main menu's File|Exit should have absolute priority over anything that happens in the terminal.
It works that way when Octave is running in a rxvt or cmd32.exe terminal as well.

A confirmation popup is a good idea.

Note that there was a change recently in order to make the exit via command line, i.e., typing "exit" in the command window, and exit via GUI, i.e., selecting File | Exit follow a consistent path when doing the shutdown sequence (i.e., check if files need to be saved, etc).  Otherwise, the coordination of such a thing is very convoluted because the GUI needs some type of state variable to decipher where the exit came from, how to stop the Octave core thread from stopping if user wants to cancel, and so on.  The consistent exit-via-command is much easier to deal with.

That said, making the user issue a Cntrl-C to exit the running command is much better than forcing the worker process to close and thereby possibly losing minutes or hours of run time.  However, the best way to do this, and it isn't difficult, just need time, would be if the GUI File | Exit event happens in addition to placing "exit" in the queue is to create a Qt timer and connect a slot to its expiration that will put up a dialog box and ask if the worker Octave thread should be interrupted thereby confirming exit in the process as well.  Set the timer for, say, three second.  The shutdown will then go forward because of the queued exit command.  So there is a little logistics to deal with, but it is all within the GUI's realm so-to-speak rather than having to balance what is going on in two threads.

I can look at this later if no one else has bandwidth, but it might be a couple days.

Confirmed here. The "exit" issued by the GUI is queued and works once the long-running command finishes.

Entering Ctrl-C in the command window also interrupts the long-running command and the exit then takes effect immediately.