bugGNU Octave - Bugs: bug #49779, print /non/existing/dir/file.svg...

 
 

bug #49779: print /non/existing/dir/file.svg crashes octave

Submitted by:  Muhali <muhali>
Submitted on:  Tue 06 Dec 2016 05:03:53 PM UTC  
 
Category: Plotting with OpenGLSeverity: 3 - Normal
Priority: 5 - NormalItem Group: Segfault, Bus Error, etc.
Status: FixedAssigned to: None
Originator Name: Open/Closed: Closed
Release: devOperating System: GNU/Linux

Add a New Comment(Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

(Jump to the original submission Jump to the original submission)

Tue 31 Jan 2017 02:30:24 PM UTC, comment #18:

Closing this report since the remaining issue is now documented in bug #50198.

Rik <rik5>
Project Administrator
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.

Pantxo Diribarne <pantxo>
Project Member
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.

John W. Eaton <jwe>
Project Administrator
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.

Pantxo Diribarne <pantxo>
Project Member
Mon 30 Jan 2017 09:04:26 PM UTC, comment #14:

@Pantxo: Yes, this can go on stable.

Rik <rik5>
Project Administrator
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)

Pantxo Diribarne <pantxo>
Project Member
Mon 30 Jan 2017 05:23:40 AM UTC, comment #12:

The patch from comment #11 attached.

(file #39609)

Avinoam Kalma <avinoam>
Project Member
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.

Avinoam Kalma <avinoam>
Project Member
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.

Pantxo Diribarne <pantxo>
Project Member
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.

Rik <rik5>
Project Administrator
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.

Pantxo Diribarne <pantxo>
Project Member
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.

Avinoam Kalma <avinoam>
Project Member
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.

Avinoam Kalma <avinoam>
Project Member
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.

Rik <rik5>
Project Administrator
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
Pantxo Diribarne <pantxo>
Project Member
Wed 07 Dec 2016 07:33:38 AM UTC, comment #3:

What about trivial check of directory existance in print.m?

Avinoam Kalma <avinoam>
Project Member
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.

Rik <rik5>
Project Administrator
Tue 06 Dec 2016 05:06:02 PM UTC, comment #1:

sorry, I meant

graphics_toolkit qt ;

Muhali <muhali>
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.

Muhali <muhali>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach File(s):
   
   
Comment:
   

Attached Files
file #39616:  print2.patch added by pantxo (2KiB - text/x-diff)
file #39609:  print.diff added by avinoam (1KiB - application/octet-stream)

 

Depends on the following items: None found

Digest:
   bug dependencies.

 

Carbon-Copy List
  • -unavailable- added by jwe (Posted a comment)
  • -unavailable- added by rik5
  • -unavailable- added by pantxo (Posted a comment)
  • -unavailable- added by avinoam (Posted a comment)
  • -unavailable- added by rik5 (Posted a comment)
  • -unavailable- added by muhali (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only project members can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 10 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Tue 31 Jan 2017 02:30:24 PM UTCrik5StatusPatch Reviewed=>Fixed
      Open/ClosedOpen=>Closed
    Mon 30 Jan 2017 08:56:04 PM UTCpantxoStatusPatch Submitted=>Patch Reviewed
    Mon 30 Jan 2017 08:55:29 PM UTCpantxoAttached File-=>Added print2.patch, #39616
      Item GroupNone=>Segfault, Bus Error, etc.
    Mon 30 Jan 2017 07:14:27 PM UTCavinoamStatusConfirmed=>Patch Submitted
    Mon 30 Jan 2017 05:23:40 AM UTCavinoamAttached File-=>Added print.diff, #39609
    Thu 26 Jan 2017 06:40:27 PM UTCrik5Dependencies-=>bugs #50146 is dependent
    Wed 07 Dec 2016 04:22:33 PM UTCrik5Carbon-Copy-=>Added jwe
    Tue 06 Dec 2016 09:17:43 PM UTCrik5StatusNone=>Confirmed

    Back to the top


    Powered by Savane 3.1-cleanup1