Tue 31 Jan 2017 02:30:24 PM UTC, comment #18:
Closing this report since the remaining issue is now documented in bug #50198.
|
Tue 31 Jan 2017 02:18:44 PM UTC, comment #17:
I posted bug #50198. The test case is somewhat artificial, since I didn't know how to force "error" to be called I added a call to error by hand.
|
Tue 31 Jan 2017 01:45:06 PM UTC, comment #16:
How is it about handling exceptions in the GUI thread?
Could you create a simplified example of just that problem and post it in a new bug report?
Thanks.
|
Mon 30 Jan 2017 09:34:47 PM UTC, comment #15:
I pushed the patch here:
http://hg.savannah.gnu.org/hgweb/octave/rev/46b914a213db
@Rik: I'll let you decide if this report should stay open and where to discuss the real deal, handling exceptions in the GUI thread.
|
Mon 30 Jan 2017 09:04:26 PM UTC, comment #14:
@Pantxo: Yes, this can go on stable.
|
Mon 30 Jan 2017 08:55:29 PM UTC, comment #13:
Thanks Avinoam, it works form me. I attached a modified hg changeset. The modifications are:
- added the change log
- added spaces before parenthesis in function call
- added spaces after ! operator
- "error" accepts the same (template, arguments) structure as "sprintf" so I simplified this,
@Rik: should this go on stable since it avoids an easily triggered segfault?
(file #39616)
|
Mon 30 Jan 2017 05:23:40 AM UTC, comment #12:
The patch from comment #11 attached.
(file #39609)
|
Mon 30 Jan 2017 05:18:53 AM UTC, comment #11:
Please review the following diff.
The changes in print.m are according to comment #10.
I have also added another input check in /__print_parse_opts__.
I hope it is correct and I did not miss something.
|
Sat 28 Jan 2017 02:04:35 PM UTC, comment #10:
@Rik: I think bug #44387 is the place where the GUI exception handler could be discussed
@Avinoam: could you put your snippet from comment #7 into a patch with a few modifications:
- use double quotes
- use ! instead of ~
- delete the file after fclose (there may be another error afterward, in which case we don't wan't this file to be leftover)
After that we will still need to handle non implemented devices.
|
Fri 27 Jan 2017 04:55:06 PM UTC, comment #9:
@Pantxo: I'm a big fan of the 80/20 rule, i.e., you can get 80% of what you want with just a 20% effort. That seems to be the case here, and I don't have problems adding another input validation test.
However, we still need to keep a bug report open somewhere (maybe a new one with a clear title) that documents the root problem. Otherwise we will keep encountering this bug and solving different aspects of it in different files until the code base because littered with workarounds.
|
Fri 27 Jan 2017 10:41:08 AM UTC, comment #8:
@Rik: I agree that in many cases using exception instead of carefully checking input is a simpler approach, but in this case, even when we manage to rethrow exceptions from the GUI thread, it means gl2ps will spend eventually a lot of time preparing an eps that can't be written or converted. Imagine
Anyone of those print commands will lead to an error after 37s on my rather new computer when we could have had it return an error early.
|
Fri 27 Jan 2017 06:48:56 AM UTC, comment #7:
What about the following check:
It is close to Pantxo suggestion (comment #4), and solves this bug and bug #50146. I know that this not the desired solutions, and does not solve other expected and unexpected situations, it can
give an interim fix.
|
Mon 09 Jan 2017 05:52:57 AM UTC, comment #6:
I believe that 90% of the crashes will be because of a wrong
directory, so I still think that my trivial patch from comment #3 should be applied, and another exception mechanism that will treat
other expected and unexpected situations should be added later.
|
Wed 07 Dec 2016 04:22:33 PM UTC, comment #5:
My concern is that we spend a lot of time coding ever more elaborate input validation tests which still manage to miss some cases. The whole point of exception handling is being able to handle unexpected situations.
From Stroustroup himself at http://www.stroustrup.com/C++11FAQ.html.
Maybe it would take just as much coder time to do this correctly. I'm adding jwe to the CC list since I don't have a great understanding of the cross-thread issues going on with the GUI.
|
Wed 07 Dec 2016 03:04:44 PM UTC, comment #4:
Yes I think Avinoam is right. Since this crash can be triggered by a simple typo, checking print options early is the right way to go for the time being:
- check the destination directory exists
- check the user has write access to this directory (open a temporary file for writting there and delete it immediately)
- check the print device is correct: a typo like "-depcs" instead of "-depsc" also crashes Octave
|
Wed 07 Dec 2016 07:33:38 AM UTC, comment #3:
What about trivial check of directory existance in print.m?
|
Tue 06 Dec 2016 09:17:43 PM UTC, comment #2:
Confirmed. This bug has been reported in other guises. The issue is that the GUI has multiple threads with one for Qt plotting and one for Octave processing. When using C++ exceptions in the Qt thread the system libraries are not channeling the exception to the handler that the Octave main thread has installed. Instead, no handler is registered so the default terminate function is called which stops the program.
This is a nasty but of cross-thread programming that I don't know how to resolve just yet.
|
Tue 06 Dec 2016 05:06:02 PM UTC, comment #1:
sorry, I meant
graphics_toolkit qt ;
|
Tue 06 Dec 2016 05:03:53 PM UTC, original submission:
By doing
one makes octave-dev crash, like so:
sh: /tmp/foo/foo.svg: No such file or directory
warning: broken pipe
error: gl2ps_renderer::draw: internal pipe error
terminate called after throwing an instance of 'octave::execution_exception'
panic: Aborted -- stopping myself...
attempting to save variables to 'octave-workspace'...
save to 'octave-workspace' complete
Aborted (core dumped)
The stable version just exits after the broken pipe.
|