bugGNU Octave - Bugs: bug #47381, unwind_protect_cleanup block not...

 
 

bug #47381: unwind_protect_cleanup block not run for some errors in C++ code

Submitted by:  Rik <rik5>
Submitted on:  Thu 10 Mar 2016 06:06:12 PM UTC  
 
Category: InterpreterSeverity: 4 - Important
Priority: 5 - NormalItem Group: Incorrect Result
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 22 Mar 2016 10:25:22 PM UTC, comment #6:

I pushed a cset here (http://hg.savannah.gnu.org/hgweb/octave/rev/ab8760b1245d) which clears the SIGPIPE condition and raises an error to the user. Marking as fixed and closing report.

Rik <rik5>
Project Administrator
Sat 12 Mar 2016 09:17:36 AM UTC, comment #5:

@Rik: I must say I am completely ignorant when it comes to interaction with the system and signal handling. For example I don't know what will happen when the stream is a file, not a pipe, and the memory of the drive is insufficient. Will the system throw a signal that we should also handle? Will fwrite simply not write as much characters as expected (in which case your new construct is useful anyway)?

Pantxo Diribarne <pantxo>
Project Member
Fri 11 Mar 2016 09:37:16 PM UTC, comment #4:

I'll try to look at this soon.

John W. Eaton <jwe>
Project Administrator
Thu 10 Mar 2016 07:41:54 PM UTC, comment #3:

The problem is in gl2ps_print.cc.

If the pipe is closed, because the command failed, then this code does not detect that condition nor raise an error. Instead, the call to gnulib::fwrite sends a signal SIGPIPE which is caught by Octave's signal handler over in sighandlers.cc.

Since the file being copied is quite large (potentially MB of eps data) the pipe_handler_error_count eventually exceeds 100 and Octave's interrupt state gets set. Thus, when the drawnow function call eventually returns the interpreter detects that and returns to the command line immediately bypassing the unwind_protect_cleanup blocks.

I tried this

and it seems to work. The possible changeset is attached to the report as SIGPIPE.cset, and needs review.

One thing I notice is that this sequence works.

But, it produces the extra message "warning: broken pipe". This comes from octave_signal_handler in sighandlers.cc. It is a minor quibble, but maybe it indicates that the patch should take a different direction. Instead of calling error when a bad pipe condition is detected it could call octave_signal_handler. That would reset the state of SIGPIPE, as well as print the warning message correctly attached to the command that caused it.

Or maybe we should be setting up Octave to ignore SIGPIPE signals when running this code. That sounds like more work, and I don't know exactly how to do it.

(file #36599)

Rik <rik5>
Project Administrator
Thu 10 Mar 2016 07:29:22 PM UTC, comment #2:

Adding jwe and panxto to the CC list for review.

Rik <rik5>
Project Administrator
Thu 10 Mar 2016 06:24:55 PM UTC, comment #1:

Small correction. It is the call to drawnow, not _osmesa_print_ which is the problem.

Rik <rik5>
Project Administrator
Thu 10 Mar 2016 06:06:12 PM UTC, original submission:

The purpose of the unwind_protect/unwind_protect_cleanup construct is to be able to trap errors and recover if necessary. Unfortunately, I have discovered at least one exception to this when printing.

I've constructed a test script which tests a path of multiple unwind blocks.

When run without the call to print I see that each cleanup block is executed.

However, when I uncomment the line which calls print I get

The ordering of message in stdout is slightly messed up, but you can see that no cleanup blocks are ever executed.

This isn't about merely having C++ code. I used mkoctfile to build a DLDFCN which throws an error. Calling that function in place of print works correctly.

The first problem is why the Octave interpreter is not working correctly when print eventually invokes _osmesa_print_ at the bottom of _opengl_print_.m. The second question is whether there are other instances where this might happen such as GraphicsMagick or other DLDFCN libraries.

Incidentally, I tested with 'graphics_toolkit gnuplot' and then the script passes.

Rik <rik5>
Project Administrator

 

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

Attach File(s):
   
   
Comment:
   

Attached Files
file #36599:  SIGPIPE.cset added by rik5 (1KiB - application/octet-stream)
file #36595:  tst_mult_unwind.m added by rik5 (694B - d2l/unknowntype)
file #36596:  throwerr.cc added by rik5 (318B - text/x-c)
file #36597:  tst_unwind.m added by rik5 (218B - d2l/unknowntype)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by pantxo (Posted a comment)
  • -unavailable- added by jwe (Posted a comment)
  • -unavailable- added by rik5
  • -unavailable- added by rik5
  • -unavailable- added by rik5 (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 8 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Tue 22 Mar 2016 10:25:22 PM UTCrik5StatusNone=>Fixed
      Open/ClosedOpen=>Closed
    Thu 10 Mar 2016 07:41:54 PM UTCrik5Attached File-=>Added SIGPIPE.cset, #36599
    Thu 10 Mar 2016 07:29:22 PM UTCrik5Carbon-Copy-=>Added jwe
      Carbon-Copy-=>Added pantxo
    Thu 10 Mar 2016 06:06:12 PM UTCrik5Attached File-=>Added tst_mult_unwind.m, #36595
      Attached File-=>Added throwerr.cc, #36596
      Attached File-=>Added tst_unwind.m, #36597

    Back to the top


    Powered by Savane 3.1-cleanup1