Mon 20 Nov 2017 10:31:31 PM UTC, comment #26:
I think we could use the Qt::BlockingQueuedConnection if the signal were originating from the main window of the GUI. So perhaps what we need is something like
command_interpreter SIGNAL octave_finished_signal
goes to
main_window SLOT (initiate_quit???)
and the the main window has a connection
main_window, SIGNAL (quitmainthread???)
goes to
main_thread SLOT (quit ()) with Qt::BlockingQueuedConnection
That is, the main_window is the one that always quits the main_thread and then waits on it.
|
Mon 20 Nov 2017 10:23:36 PM UTC, comment #25:
I may have placed the connection classification on the wrong signal/slot pair. Not sure. The problem is that it's now the interpreter issuing this signal (octave_finished_signal (int)), but the interpreter lives in the main_thread (via move) so I'm not sure if Qt::BlockingQueuedConnection even applies anymore because signal/slot in the same thread are not queued.
So, it may be that we have to go back to a setup similar to the original patch in which the quit/wait is done via the main window.
(file #42446)
|
Mon 20 Nov 2017 10:03:05 PM UTC, comment #24:
It looks as though on the stable branch that JWE shuffled some code around and associated the m_main_thread with the octave_qt_link object. Hence m_main_thread is no longer accessible.
So, I've tried the idea from Panxto, which is to make the connection between finish() and deleteLater() blocking, which I'm hoping is essentially the same this as waiting. So, here's a patch to try for the stable branch. And perhaps if that works in stable it can easily be incorporated in default branch.
(file #42445)
|
Mon 20 Nov 2017 09:34:25 PM UTC, comment #23:
jwe: I made a fresh checkout of the stable branch and compiled it (it still calls itself 4.2.1, right?). Octave still freezes on exit if one has plotted before.
|
Mon 20 Nov 2017 08:53:15 PM UTC, comment #22:
Sebastian: I'd like to know what happens with the current stable branch sources without any patches applied.
|
Mon 20 Nov 2017 08:40:40 PM UTC, comment #21:
>Sebastian, we are wondering if the current development branch has the issue with
> freezing at exit in MacOS corrected or if there still needs to be some kind of patch.
> Would it be easy for you to test the most recent development branch that is going to
> form the basis for 4.2.2?
Sure. Will do. However, I assume Mike was rather sceptical that your patch can be backported (see comment #16).
|
Mon 20 Nov 2017 08:31:52 PM UTC, comment #20:
Sebastian, we are wondering if the current development branch has the issue with freezing at exit in MacOS corrected or if there still needs to be some kind of patch. Would it be easy for you to test the most recent development branch that is going to form the basis for 4.2.2?
|
Thu 12 Oct 2017 08:13:48 AM UTC, comment #19:
Oh yeah, you're right. I forgot that the signal type can be altered when making the Qt connection. The wait of quit/wait construct effectively creates a lock (I think, but haven't explored). Is there any difference between the two constructs? I'm not sure. Nonetheless, the quit/wait construct is an example as shown in the Qt documentation, so it's got that going for it.
I'm open to the blocking queued signal approach, though, but at the same time prefer not experimenting too much with something that seems to work, at least for now, in an otherwise historically fragile exit process. If/when the exit is overhauled to be more robust is a better time for experimenting.
|
Mon 09 Oct 2017 01:52:45 PM UTC, comment #18:
@Dan:
>> The signal/slot connection doesn't have a means for waiting
I don't know if it is relevant here since since I did not look into this at all, but ther is a way to have the signal function return only when the slot has returned. The connection must be made in queud mode:
connect (signal,..., Qt::BlockingQueuedConnection)
There is a requirement though, the signaling thread is then locked until the slot thread has returned: it means that the slot thread can't request some operation to be done in the signal thread because it would lead to a dead lock.
|
Sat 07 Oct 2017 05:55:13 PM UTC, comment #17:
Correct, in all likelihood the QThread::wait is the necessary item. The signal/slot connection doesn't have a means for waiting.
Hopefully the next Octave release isn't too far off. There have been quite a few seemingly critical bugs following the most recent release, so it would be nice to get something more stable out there.
|
Sat 07 Oct 2017 05:33:31 PM UTC, comment #16:
It's unlikely that this could be backported to the stable branch, but anyone is welcome to try. There has already been very significant refactoring work done in the GUI on the development branch before this point.
As far as applying this patch to the default branch, I see no problems with it. I suppose the relevant change here is the QThread::wait for completion before destroying the thread.
|
Tue 03 Oct 2017 02:12:01 AM UTC, comment #15:
I can't make changes in the canonical repository. It's a very small changeset, so backporting doesn't seem complicated if you think it would help. (Most of the main developers don't work much with MacOS.) However, it would probably be good to make sure several MacOS testers feel comfortable the fix works before backporting.
|
Mon 02 Oct 2017 07:15:54 PM UTC, comment #14:
I'd also be interested in seeing a backport. Last week, I ran into this issue in front of my physics class, with octave 4.2.1, built using MacPorts.
|
Sun 01 Oct 2017 06:13:20 PM UTC, comment #13:
@Dan: did you consider a backport?
|
Sun 01 Oct 2017 05:20:31 PM UTC, comment #12:
OK, thanks for the clarification. I don't know the MacOS build process, but it sounds like it takes some work and finding another user to test might be difficult. It's up to Mike how to proceed.
|
Sun 01 Oct 2017 05:03:32 PM UTC, comment #11:
>Or is it the case that you think you didn't have patch 41889 applied and built cleanly in the first place, but now it seems to work?
sorry for the confusion. I am pretty sure that my first attempt was not a clean build. Now, I have verified that both versions of your patch work for me and I always started from a fresh checkout :)
|
Sun 01 Oct 2017 04:56:25 PM UTC, comment #10:
Mike is on this thread, so he'd be the one to incorporate the patch if it works. I'm a bit unclear about which patch works because you initially wrote something about it not solving the bug. Patch 41889 and 41891 are effectively the same, the latter having a little extra debug info. So if the latter works and the former doesn't, then that is simply a matter of the extra fprintfs slowing things down and not addressing the main problem. Or is it the case that you think you didn't have patch 41889 applied and built cleanly in the first place, but now it seems to work?
|
Sun 01 Oct 2017 04:13:49 PM UTC, comment #9:
I checked on several machines with various qt and macOS versions. The patch works for me! I strongly suggest to push it to the development branch. I tried to back port it but I was not able. @Dan: would that be possible?
|
Tue 26 Sep 2017 07:31:40 AM UTC, comment #8:
Well, thread timing bugs are some of the stranger bugs. When things aren't coded according to the design, changing things slightly in either thread can "fix" things or "break" them.
It's odd that you don't see any of the fprintf(stderr,...) output. If you are using "exit" at the command line to close the GUI, it would have to go through
main_window::handle_octave_finished
and hence at least the first fprintf() output should appear. I don't know about Mac OS X, though. Perhaps stderr is directed somewhere other than the terminal window by default.
|
Tue 26 Sep 2017 07:08:55 AM UTC, comment #7:
I am very confused. I just compiled head with your patch. I see no output at all (with or without "--no-gui") but even more crazily: it does not freeze anymore.
I will try to find out what is going on later today.
Thanks,
Sebastian
P.S.: yes, I am sure that I am looking at the correct version of octave :)
|
Mon 25 Sep 2017 03:31:53 PM UTC, comment #6:
We need to par down the amount of debug info to get an idea of what's causing the crash. I'm attaching the same patch, but I put some prints within the code that should be easy to recognized even if they are mixed in with some system trace. Could you run Octave from a command line and check if there is any meaningful output at exit? First try:
./run-octave --no-gui
and see if there is an error. (Even though the option says "no-gui", the GUI is actually launched, just not displayed.) And then run
./run-octave &
(file #41891)
|
Mon 25 Sep 2017 08:42:11 AM UTC, comment #5:
Thanks Dan! I really appreciate this. The GUI is nicely working on macOS besides freezing on exit and some scaling issues of the plot engine on high-dpi screens. It seems rather minor stuff that stops mac users from using the GUI...
Sadly, your patch is not preventing that octave freezes. How can I provide helpful debug output?
Sebastian
|
Sun 24 Sep 2017 09:09:21 PM UTC, comment #4:
Let's try a simple little change (attached patch). All this does is move the thread quit() to the GUI's main window which will allow also waiting on that quit. It works here on Linux. Let me know if it works on Mac OS X. That's about all I have time to explore right now.
(file #41889)
|
Sun 24 Sep 2017 08:26:36 PM UTC, comment #3:
Folks, I think this is more of an Octave proper-Qt-programming issue than it is an OS X problem. In particular, it is the proper sequence of shutting down threads. I looked at the system complaints in the bug report
https://savannah.gnu.org/bugs/?50795
and although the dump is so voluminous to really not be helpful, I do see "pthread" and such in the list.
Let me try to illustrate what the issue probably is from looking at hunks of code.
The following is from mainwindow.cc:
What I wanted to explore is whether not starting the thread results in a clean thread. But when I comment out the m_main_thread->start() line on Linux, the GUI hangs and only kill -9 will stop it. Well, that's bad in the sense that Octave GUI should be much more graceful if the thread doesn't start properly. It doesn't seem far-fetched that a thread doesn't start, so here is another thing to look at. (My feeling is the whole startup/shutdown concept needs an overhaul.)
Now, shutdown. octave_finished_signal (probably emitted when the interpreter is about to exit) is connected to "this" (i.e., the GUI main window) handle_octave_finished(). That's fine, it's just letting the GUI know it's done.
The m_main_thread finished() signal is connected to its deleteLater() slot. This is fine too, as it has become a typical construct, at least in this prototypical example:
http://doc.qt.io/qt-5/qthread.html#details
Where I think the problem lies is this line:
The m_interpreter is running in the thread, and it tells the thread to quit. The Qt documentation makes no mention of such a construct, but I'm imagining that being an undefined sort of behavior. Let's take a look at the prototype example of how quit is used:
Notice it is the Controller, the thing that launched the worker thread, that is issuing the quit. The quit is not coming from within the thread. But more than that, and probably the key issue, is that the Controller can also wait on the thread to finish. If the GUI does not wait on the worker thread to finish, somewhere in the exit process, then there is a good chance this is problematic. I suspect that is the source of failure for Mac OS X, i.e., that threads are structured differently internal to the OS.
Notice with the signal/slot construct of accessing quit(), there is no straightforward way of also doing a wait() somewhere. This is why I'm saying the exit process needs to be reworked. Add to that the GUI should also gracefully handle the thread not starting properly. Also, the GUI should gracefully handle the interpreter crashing and the interpreter not responding at exit. There is just a whole lot of work that needs to be done.
|
Fri 27 Jan 2017 07:37:16 AM UTC, comment #2:
Yes. Sadly, this is known. However, it might be that there was no bug report until now. This problem is there since we moved to qt5. I guess jpswensen is currently the only one who might be able to help here (see #49053).
|
Fri 27 Jan 2017 01:48:42 AM UTC, comment #1:
Adding sschoeps in cc. As far as I know it is known that the GUI can crash and freeze several different ways on macOS. If you can help debug and help make Octave more stable, please do so.
|
Wed 11 Jan 2017 02:08:30 PM UTC, original submission:
I have installed Octave version 4.2.0 via homebrew and created an Apple scriptfile with which I can start Octave Graphical User Interface.
But when I quit Octave GUI with simply closing it by clicking 'quit octave-gui' or typing 'quit' in command line, Octave shut down as it should but the window freezes and doesn't close until I force it to close with Apples Activity Manager.
|