bugGNU Octave - Bugs: bug #48640, gzip/bzip errors in destructors...

 
 

bug #48640: gzip/bzip errors in destructors will cause Octave to crash

Submitter:  Mike Miller <mtmiller>
Submitted:  Tue 26 Jul 2016 07:08:08 PM UTC
   
 
Category:  Octave Function Severity:  4 - Important
Priority:  5 - Normal Item Group:  Segfault, Bus Error, etc.
Status:  Fixed Assigned to:  None
Originator Name:  Open/Closed:  * Closed
Release:  * dev Operating System:  * Any
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Wed 27 Jul 2016 09:26:30 PM UTC, comment #4: 
Carnë Draug <carandraug>
Group Member
Wed 27 Jul 2016 09:10:46 PM UTC, comment #3: 

Looks good to me, this change gets rid of the GCC warning messages and makes sense to me.

Mike Miller <mtmiller>
Group Member
Wed 27 Jul 2016 03:43:24 PM UTC, comment #2: 

Could you review this change?

  https://bitbucket.org/carandraug/octave/commits/e53bd08a19e0868eee2c4237e8212f0a16d468b2?at=bug-48640

I went with the use of a close method instead of warnings since warnings wouldn't add anything useful.  The only error handling this function should do is skip files that caused errors and not include them in the list of output files.

I also realized that if the destructor is called as part of exception handling, we have nothing to do about a new error. So we just do our best to free any resources.  In other cases, then we call close() which would throw as expected.

Carnë Draug <carandraug>
Group Member
Tue 26 Jul 2016 07:09:42 PM UTC, comment #1: 

Cc carandraug, convert to warnings as in http://hg.savannah.gnu.org/hgweb/octave/rev/63c806042c27, or handle stream termination errors some other way?

Mike Miller <mtmiller>
Group Member
Tue 26 Jul 2016 07:08:08 PM UTC, original submission:  

The recent change to move gzip and bzip to C++ adds some classes to handle the various compression streams. The destructors throw runtime_error exceptions, but these should be changed into warnings instead of errors. As it is now, these throws will simply call terminate and Octave will crash immediately.

According to http://en.cppreference.com/w/cpp/language/destructor#Exceptions, throwing exceptions from a destructor is disabled by default, unless the destructor is declared "noexcept(false)", but is bad practice in general.

GCC 6 emits the warnings:


../libinterp/dldfcn/xzip.cc: In destructor ‘CFile::~CFile()’:
../libinterp/dldfcn/xzip.cc:92:55: warning: throw will always call terminate() [-Wterminate]
       throw std::runtime_error ("unable to close file");
                                                       ^
../libinterp/dldfcn/xzip.cc:92:55: note: in C++11 destructors default to noexcept
../libinterp/dldfcn/xzip.cc: In destructor ‘bz2::zipper::~zipper()’:
../libinterp/dldfcn/xzip.cc:140:65: warning: throw will always call terminate() [-Wterminate]
         throw std::runtime_error ("failed to close bzip2 stream");
                                                                 ^
../libinterp/dldfcn/xzip.cc:140:65: note: in C++11 destructors default to noexcept
../libinterp/dldfcn/xzip.cc: In destructor ‘gz::gzip_stream::~gzip_stream()’:
../libinterp/dldfcn/xzip.cc:219:64: warning: throw will always call terminate() [-Wterminate]
         throw std::runtime_error ("failed to close zlib stream");
                                                                ^
../libinterp/dldfcn/xzip.cc:219:64: note: in C++11 destructors default to noexcept


Mike Miller <mtmiller>
Group Member

 

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

Attach Files:
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by mtmiller
  • -email is unavailable- added by mtmiller (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only group members can vote.

     

    Follow 4 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2016-07-27 carandraug StatusPatch Submitted Fixed
        Open/ClosedOpen Closed
    2016-07-27 mtmiller StatusNone Patch Submitted
    2016-07-26 mtmiller Carbon-Copy- Added carandraug

    Back to the top

    Powered by Savane 3.13-02a9.
    Corresponding source code