Boost C++ Libraries: Ticket #3650: Text Serialization crashes when istream close unexpectedly

component changed; owner set

Steven Watanabe — Fri, 20 Nov 2009 20:37:59 GMT

owner set to Robert Ramey
component None → serialization

Robert Ramey — Sat, 28 Nov 2009 18:55:03 GMT

Hmmm - I'm not sure that is the correct fix.

wouldn't it be better to use something like:

boost::archive::text_iarchive ia(*_stream); while(! _stream->eof() && ! _stream->fail() ) {

boost::shared_ptr<Message> msg; ia >> msg; ...

}

I'm reluctant to add "too much" code in the depths of the library.

Robert Ramey

Doug Wallas — Mon, 30 Nov 2009 11:28:10 GMT

The problem seems to be on the >> operator. The library do read from the stream, but does not check after reading that the stream had not encoutered an error while reading. The >> operator then starts creating a new object from what he just read and sigfaults :(

I may be wrong, but i think the fail() is updated after a read, so the check on _stream->fail() would be ok (the library already does one in internal) and the crash would still occur.

status, type changed; resolution set

Robert Ramey — Mon, 30 Nov 2009 17:27:03 GMT

status new → closed
type Bugs → Feature Requests
resolution → wontfix

The reason that I did it the way I did was to permit the user code to handle the fail case. If I throw an exception after each operation, then the user can't make code like

boost::archive::text_iarchive ia(*_stream);
do{
    boost::shared_ptr<Message> msg;
    ia >> msg; ...
}while(! _stream->eof() && ! _stream->fail() ) {

But would be obligated to write exception handling code. If you really want things to operate this way, you can write

boost::archive::text_iarchive ia(*_stream);
do{
    boost::shared_ptr<Message> msg;
    ia >> msg; ...
    if(_stream->fail())
        throw ...
}while(! _stream->eof()) {

So the current situation leaves more flexibility to the user.

Robert Ramey

anonymous — Tue, 01 Dec 2009 14:25:59 GMT

Hello, I understand your point but the library sigfaults on the >> operator... So i think it's not possible for the user code to handle it :(

 boost::archive::text_iarchive ia(*_stream);
 do{
     boost::shared_ptr<Message> msg;
     ia >> msg; // sigfaults
     if(_stream->fail()) // Wont be called
         throw ...
 }while(! _stream->eof()) {

Thanks for your time looking at the problem.

Replying to ramey:

The reason that I did it the way I did was to permit the user code to handle the fail case. If I throw an exception after each operation, then the user can't make code like
boost::archive::text_iarchive ia(*_stream);
do{
    boost::shared_ptr<Message> msg;
    ia >> msg; ...
}while(! _stream->eof() && ! _stream->fail() ) {
But would be obligated to write exception handling code. If you really want things to operate this way, you can write
boost::archive::text_iarchive ia(*_stream);
do{
    boost::shared_ptr<Message> msg;
    ia >> msg; ...
    if(_stream->fail())
        throw ...
}while(! _stream->eof()) {
So the current situation leaves more flexibility to the user.

Robert Ramey

status, type changed; resolution deleted

anonymous — Fri, 04 Dec 2009 06:46:33 GMT

status closed → reopened
type Feature Requests → Bugs
resolution wontfix

I just seen that the type was changed to feature request... The serialization library just crashes i dont think it can be considered as a feature request...

status changed; resolution set

Robert Ramey — Sun, 06 Dec 2009 22:12:33 GMT

status reopened → closed
resolution → wontfix

 boost::archive::text_iarchive ia(*_stream);
 do{
     boost::shared_ptr<Message> msg;
     // check here that the stream is OK. That is, if the stream
     // has been unexpected closed - deal with it here!
     if(_stream->eof() or stream-> ?)
         break;
     ia >> msg; // sigfaults
     if(_stream->fail()) // Wont be called
         throw ...
 }while(! _stream->eof()) {

status changed; resolution deleted

anonymous — Mon, 07 Dec 2009 13:14:02 GMT

status closed → reopened
resolution wontfix

It wouldnt work since ">>" is blocking so your checks could be OK, but it would still sigfaults on the ">>" operand as i mentioned before.

If the stream is closed while waiting on the >> operand, a modification of the library is necessary or there's no other way (i could think of) to catch that kind of error.

status, type changed; resolution set

Robert Ramey — Wed, 30 Dec 2009 18:11:14 GMT

status reopened → closed
type Bugs → Feature Requests
resolution → wontfix

The library has to assume that the stream remains open while an archive is being saved. This is not an unreasonable expectation.

basically the best way to handle this would be to trap the situation where the stream closes and not make any serialization library calls when this is detected.

In any case, the library won't be changing.

Robert Ramey

status changed; resolution deleted

Vladimir Prus — Thu, 31 Dec 2009 08:44:13 GMT

status closed → reopened
resolution wontfix

Robert, I don't understand your position here. The stream can unexpectedly fail at any moment, and stream do fail in properly written programs. Even if the user checks that stream is OK right before calling operator<<, you can get a stream error -- e.g. a network connection can close immediately after user's check is executed. It does not seem that crashing in face of a natural situation is desirable. Could you clarify further?

Thanks, Volodya

Francois Valette — Thu, 21 Jan 2010 07:46:33 GMT

Hello, We encounter the same problem sometimes when our stream failed unexpectedly. The occurence is pretty rare so it was hard to find the cause... Thx for pointing out the fix Doug.