[serialization] Reading newer archives - was throwing unsupported_class_version, now crashes
Hi Robert, I remember that in the past the serialization library was throwing unsupported_class_version exception when reading of archives with future class versions. This no longer seems to be the case. We had older version of software crash when reading newer archive. The only place that I found to mention unsupported_class_version is commented out and I do not understand the attached note: BOOST_DLLEXPORT void iserializer<Archive, T>::load_object_data( basic_iarchive & ar, void *x, const unsigned int file_version ) const { // note: we now comment this out. Before we permited archive // version # to be very large. Now we don't. To permit // readers of these old archives, we have to suppress this // code. Perhaps in the future we might re-enable it but // permit its suppression with a runtime switch. #if 0 // trap case where the program cannot handle the current version if(file_version > static_cast<const unsigned int>(version())) boost::serialization::throw_exception( archive::archive_exception( boost::archive::archive_exception::unsupported_class_version, get_debug_info() ) ); #endif // make sure call is routed through the higest interface that might // be specialized by the user. boost::serialization::serialize_adl( boost::serialization::smart_cast_reference<Archive &>(ar), * static_cast<T *>(x), file_version ); } To allow class to read serialized objects of newer version by default seems very dangerous. I generally requires extra work from the class author and without it will most likely crash. So isn't it better to have an additional trait that a class author needs to define, if a class should be allowed read future versions? something like namespace boost{ namespace serialization { template<class T> struct allow_future_versions : mpl::false_ {}; }} #define BOOST_SERIALIZATION_ALLOW_FUTURE_VERSIONS(T) \ namespace boost{ namespace serialization \ { \ template<> struct allow_future_versions<T> : mpl::true_ {}; \ }} and change the code above to: BOOST_DLLEXPORT void iserializer<Archive, T>::load_object_data( basic_iarchive & ar, void *x, const unsigned int file_version ) const { // trap case where the program cannot handle the current version if( ! boost::serialization::allow_future_versions<T>::value && file_version > static_cast<const unsigned int>(version())) boost::serialization::throw_exception( archive::archive_exception( boost::archive::archive_exception::unsupported_class_version, get_debug_info() ) ); ...... Thanks in advance, Nikolay Mladenov Sitius Automation Inc.
Nikolay Mladenov wrote:
Hi Robert,
I remember that in the past the serialization library was throwing unsupported_class_version exception when reading of archives with future class versions. This no longer seems to be the case. We had older version of software crash when reading newer archive.
The only place that I found to mention unsupported_class_version is commented out and I do not understand the attached note:
BOOST_DLLEXPORT void iserializer<Archive, T>::load_object_data( basic_iarchive & ar, void *x, const unsigned int file_version ) const { // note: we now comment this out. Before we permited archive // version # to be very large. Now we don't. To permit // readers of these old archives, we have to suppress this // code. Perhaps in the future we might re-enable it but // permit its suppression with a runtime switch. #if 0 // trap case where the program cannot handle the current version if(file_version > static_cast<const unsigned int>(version())) boost::serialization::throw_exception( archive::archive_exception(
boost::archive::archive_exception::unsupported_class_version, get_debug_info() ) ); #endif // make sure call is routed through the higest interface that might // be specialized by the user. boost::serialization::serialize_adl( boost::serialization::smart_cast_reference<Archive &>(ar), * static_cast<T *>(x), file_version ); }
To allow class to read serialized objects of newer version by default seems very dangerous. I generally requires extra work from the class author and without it will most likely crash.
So isn't it better to have an additional trait that a class author needs to define, if a class should be allowed read future versions?
something like
namespace boost{ namespace serialization { template<class T> struct allow_future_versions : mpl::false_ {}; }}
#define BOOST_SERIALIZATION_ALLOW_FUTURE_VERSIONS(T) \ namespace boost{ namespace serialization \ { \ template<> struct allow_future_versions<T> : mpl::true_ {}; \ }}
and change the code above to:
BOOST_DLLEXPORT void iserializer<Archive, T>::load_object_data( basic_iarchive & ar, void *x, const unsigned int file_version ) const { // trap case where the program cannot handle the current version if( ! boost::serialization::allow_future_versions<T>::value && file_version > static_cast<const unsigned int>(version())) boost::serialization::throw_exception( archive::archive_exception(
boost::archive::archive_exception::unsupported_class_version, get_debug_info() ) );
......
Your right about this and your proposal is a worthy one though I think there's a simpler way to do it.. I'll look into this. As to why it's this way, the answer is pretty simple. The original version of the library has some errors. Problem is, fixing them is much harder than with other libraries because of the requirement to maintain the ability to read archives created with previous versions. Robert Ramey
Thanks in advance,
Nikolay Mladenov Sitius Automation Inc.
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
On Thu, May 10, 2012 at 10:06:42PM -0800, Robert Ramey wrote:
Nikolay Mladenov wrote: As to why it's this way, the answer is pretty simple. The original version of the library has some errors. Problem is, fixing them is much harder than with other libraries because of the requirement to maintain the ability to read archives created with previous versions.
This is not an answer. Why was this code defined out, and what is solved by doing so? What _is_ the underlying problem, there is nothing in your message that explains why this happens. No-one goes around and disables code blocks for no reason, removing functionality. I cannot trust a library that has secret known bugs, especially if it has a track record of messing up (like the forgotten version bump and all that). -- Lars Viklund | zao@acc.umu.se
Lars Viklund wrote:
On Thu, May 10, 2012 at 10:06:42PM -0800, Robert Ramey wrote:
Nikolay Mladenov wrote: As to why it's this way, the answer is pretty simple. The original version of the library has some errors. Problem is, fixing them is much harder than with other libraries because of the requirement to maintain the ability to read archives created with previous versions.
This is not an answer.
lol - I believe that the question was why this is the way it is. This is the simple answer.
Why was this code defined out, and what is solved by doing so?
If I recall correctly, the problem arose when a user started overload the version number which with other data - some sort of guid or date or something like that. The version number was defined as an integer (or maybe short integer) which these days in C++ is large enough to hold this kind of data. When made a change in the library to enforce catch more user errors, this problem came up. So I commented out the change. I believe that is how we got here today. (I could be wrong about this as I'm speaking from memory). So the current complaint is that the current library doesn't detect a particular type of user error which in principle it can but in practice never has. I could just change the 0 to 1 to re-enable the code but of course I'll then risk the ire of someone who has overloaded the version data type.
What _is_ the underlying problem, there is nothing in your message that explains why this happens.
The real underlying problem is the usage in C++ of int or short int as a datatype. I've come to believe that should never be done and is always a potential bug. I've resolved to avoid doing this in the future. In fact - this problem arose because of an attempt to improve the code by implementing this policy.
No-one goes around and disables code blocks for no reason, removing functionality.
lol - I had a reason.
I cannot trust a library that has secret known bugs, especially if it has a track record of messing up (like the forgotten version bump and all that).
lol - if anyone thinks they can do a better job - feel free to step up! Robert Ramey
On Fri, May 11, 2012 at 2:06 AM, Robert Ramey <ramey@rrsd.com> wrote:
Nikolay Mladenov wrote:
Hi Robert,
I remember that in the past the serialization library was throwing unsupported_class_version exception when reading of archives with future class versions. This no longer seems to be the case. We had older version of software crash when reading newer archive.
The only place that I found to mention unsupported_class_version is commented out and I do not understand the attached note:
BOOST_DLLEXPORT void iserializer<Archive, T>::load_object_data( basic_iarchive & ar, void *x, const unsigned int file_version ) const { // note: we now comment this out. Before we permited archive // version # to be very large. Now we don't. To permit // readers of these old archives, we have to suppress this // code. Perhaps in the future we might re-enable it but // permit its suppression with a runtime switch. #if 0 // trap case where the program cannot handle the current version if(file_version > static_cast<const unsigned int>(version())) boost::serialization::throw_exception( archive::archive_exception(
boost::archive::archive_exception::unsupported_class_version, get_debug_info() ) ); #endif // make sure call is routed through the higest interface that might // be specialized by the user. boost::serialization::serialize_adl( boost::serialization::smart_cast_reference<Archive &>(ar), * static_cast<T *>(x), file_version ); }
To allow class to read serialized objects of newer version by default seems very dangerous. I generally requires extra work from the class author and without it will most likely crash.
So isn't it better to have an additional trait that a class author needs to define, if a class should be allowed read future versions?
something like
namespace boost{ namespace serialization { template<class T> struct allow_future_versions : mpl::false_ {}; }}
#define BOOST_SERIALIZATION_ALLOW_FUTURE_VERSIONS(T) \ namespace boost{ namespace serialization \ { \ template<> struct allow_future_versions<T> : mpl::true_ {}; \ }}
and change the code above to:
BOOST_DLLEXPORT void iserializer<Archive, T>::load_object_data( basic_iarchive & ar, void *x, const unsigned int file_version ) const { // trap case where the program cannot handle the current version if( ! boost::serialization::allow_future_versions<T>::value && file_version > static_cast<const unsigned int>(version())) boost::serialization::throw_exception( archive::archive_exception(
boost::archive::archive_exception::unsupported_class_version, get_debug_info() ) );
......
Your right about this and your proposal is a worthy one though I think there's a simpler way to do it.. I'll look into this.
As to why it's this way, the answer is pretty simple. The original version of the library has some errors. Problem is, fixing them is much harder than with other libraries because of the requirement to maintain the ability to read archives created with previous versions.
I am not sure how to understand this. The commented out code checks for future versions, not for previous versions?
Robert Ramey
Thanks in advance,
Nikolay Mladenov Sitius Automation Inc.
If I am to avoid the crash when reading future version I seem to have 2 options: 1) add a check in every load/serialize function and throw when future version is detected 2) add the same check but in the fuction or functions that call the load/serialize functions My problem with 1 is that there are many calls that need to be fixed, plus a new guideline is needed for future serializable classes. Not to mention that this problem applies to other people and companies (https://svn.boost.org/trac/boost/ticket/6856) And my problem with 2 is that I have to mess up with the serialization library which I am not so familiar with. Nikolay Mladenov Sitius Automation Inc.
Nikolay Mladenov wrote:
On Fri, May 11, 2012 at 2:06 AM, Robert Ramey <ramey@rrsd.com> wrote:
Your right about this and your proposal is a worthy one though I think there's a simpler way to do it.. I'll look into this.
As to why it's this way, the answer is pretty simple. The original version of the library has some errors. Problem is, fixing them is much harder than with other libraries because of the requirement to maintain the ability to read archives created with previous versions.
I am not sure how to understand this. The commented out code checks for future versions, not for previous versions?
Robert Ramey
Thanks in advance,
Nikolay Mladenov Sitius Automation Inc.
If I am to avoid the crash when reading future version I seem to have 2 options: 1) add a check in every load/serialize function and throw when future version is detected 2) add the same check but in the fuction or functions that call the load/serialize functions
3) just re-enable the check by chaning the 0 to 1 and rebuild the library. would this not work for you? Robert Ramey
And my problem with 2 is that I have to mess up with the serialization library which I am not so familiar with.
Nikolay Mladenov Sitius Automation Inc.
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
On Fri, May 11, 2012 at 1:23 PM, Robert Ramey <ramey@rrsd.com> wrote:
Nikolay Mladenov wrote:
On Fri, May 11, 2012 at 2:06 AM, Robert Ramey <ramey@rrsd.com> wrote:
Your right about this and your proposal is a worthy one though I think there's a simpler way to do it.. I'll look into this.
As to why it's this way, the answer is pretty simple. The original version of the library has some errors. Problem is, fixing them is much harder than with other libraries because of the requirement to maintain the ability to read archives created with previous versions.
I am not sure how to understand this. The commented out code checks for future versions, not for previous versions?
Robert Ramey
Thanks in advance,
Nikolay Mladenov Sitius Automation Inc.
If I am to avoid the crash when reading future version I seem to have 2 options: 1) add a check in every load/serialize function and throw when future version is detected 2) add the same check but in the fuction or functions that call the load/serialize functions
3) just re-enable the check by chaning the 0 to 1 and rebuild the library. would this not work for you?
I am not sure, but if you say so, it probably would. May I also commit, so I don't have to do this next time we upgrade boost? Nikolay
Robert Ramey
And my problem with 2 is that I have to mess up with the serialization library which I am not so familiar with.
Nikolay Mladenov Sitius Automation Inc.
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Hi Robert, I have been reading the recent thread "Boost and Exceptions" with great interest. There you discuss at some length the problems of introducing breaking changes and library authoring in general. This reminded me that you have not yet adequately responded to the problem posted in thread, so I decided to bump it up. Do you have any plans about acknowledging/fixing the problem ( a breaking change) discussed here and filed as ticked: https://svn.boost.org/trac/boost/ticket/6856. Thanks in advance, Nikolay Mladenov Sitius Automation On Fri, May 11, 2012 at 3:13 PM, Nikolay Mladenov < nikolay.mladenov@gmail.com> wrote:
On Fri, May 11, 2012 at 1:23 PM, Robert Ramey <ramey@rrsd.com> wrote:
Nikolay Mladenov wrote:
On Fri, May 11, 2012 at 2:06 AM, Robert Ramey <ramey@rrsd.com> wrote:
Your right about this and your proposal is a worthy one though I think there's a simpler way to do it.. I'll look into this.
As to why it's this way, the answer is pretty simple. The original version of the library has some errors. Problem is, fixing them is much harder than with other libraries because of the requirement to maintain the ability to read archives created with previous versions.
I am not sure how to understand this. The commented out code checks for future versions, not for previous versions?
Robert Ramey
Thanks in advance,
Nikolay Mladenov Sitius Automation Inc.
If I am to avoid the crash when reading future version I seem to have 2 options: 1) add a check in every load/serialize function and throw when future version is detected 2) add the same check but in the fuction or functions that call the load/serialize functions
3) just re-enable the check by chaning the 0 to 1 and rebuild the library. would this not work for you?
I am not sure, but if you say so, it probably would. May I also commit, so I don't have to do this next time we upgrade boost?
Nikolay
Robert Ramey
And my problem with 2 is that I have to mess up with the serialization library which I am not so familiar with.
Nikolay Mladenov Sitius Automation Inc.
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Nikolay Mladenov wrote:
Hi Robert,
I have been reading the recent thread "Boost and Exceptions" with great interest. There you discuss at some length the problems of introducing breaking changes and library authoring in general.
This reminded me that you have not yet adequately responded to the problem posted in thread, so I decided to bump it up.
Do you have any plans about acknowledging/fixing the problem ( a breaking change) discussed here and filed as ticked: https://svn.boost.org/trac/boost/ticket/6856.
Thanks in advance,
Nikolay Mladenov Sitius Automation
On Fri, May 11, 2012 at 3:13 PM, Nikolay Mladenov < nikolay.mladenov@gmail.com> wrote:
On Fri, May 11, 2012 at 1:23 PM, Robert Ramey <ramey@rrsd.com> wrote:
Nikolay Mladenov wrote:
On Fri, May 11, 2012 at 2:06 AM, Robert Ramey <ramey@rrsd.com> wrote:
Your right about this and your proposal is a worthy one though I think there's a simpler way to do it.. I'll look into this.
As to why it's this way, the answer is pretty simple. The original version of the library has some errors. Problem is, fixing them is much harder than with other libraries because of the requirement to maintain the ability to read archives created with previous versions.
I am not sure how to understand this. The commented out code checks for future versions, not for previous versions?
Robert Ramey
Thanks in advance,
Nikolay Mladenov Sitius Automation Inc.
If I am to avoid the crash when reading future version I seem to have 2 options: 1) add a check in every load/serialize function and throw when future version is detected 2) add the same check but in the fuction or functions that call the load/serialize functions
3) just re-enable the check by chaning the 0 to 1 and rebuild the library. would this not work for you?
I am not sure, but if you say so, it probably would. May I also commit, so I don't have to do this next time we upgrade boost?
Nikolay
Robert Ramey
And my problem with 2 is that I have to mess up with the serialization library which I am not so familiar with.
Nikolay Mladenov Sitius Automation Inc.
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Nikolay Mladenov wrote:
Hi Robert,
I have been reading the recent thread "Boost and Exceptions" with great interest. There you discuss at some length the problems of introducing breaking changes and library authoring in general.
This reminded me that you have not yet adequately responded to the problem posted in thread, so I decided to bump it up.
Do you have any plans about acknowledging/fixing the problem ( a breaking change) discussed here and filed as ticked: https://svn.boost.org/trac/boost/ticket/6856.
Hmmm - as I understand it, re-enableing the version # code would address your concern. Am I correct? I don't have object to doing this - the only reason I disabled it was that certain users had decided to overload the version # with other information like date. This came up as a problem when I change the size of the version # in the binary file. I suppose we can re-enable it now since this has likely been addressed by all users. Let me know if that would address your issue. And re-enable it in your own copy of the library and verify that reallly IS the issue. Robert Ramey
Thanks for the response, We have re-enabled this code in our company code and it does fix the problem. Thanks! On Mon, Jun 25, 2012 at 3:38 PM, Robert Ramey <ramey@rrsd.com> wrote:
Nikolay Mladenov wrote:
Hi Robert,
I have been reading the recent thread "Boost and Exceptions" with great interest. There you discuss at some length the problems of introducing breaking changes and library authoring in general.
This reminded me that you have not yet adequately responded to the problem posted in thread, so I decided to bump it up.
Do you have any plans about acknowledging/fixing the problem ( a breaking change) discussed here and filed as ticked: https://svn.boost.org/trac/boost/ticket/6856.
Hmmm - as I understand it, re-enableing the version # code would address your concern. Am I correct? I don't have object to doing this - the only reason I disabled it was that certain users had decided to overload the version # with other information like date. This came up as a problem when I change the size of the version # in the binary file. I suppose we can re-enable it now since this has likely been addressed by all users. Let me know if that would address your issue. And re-enable it in your own copy of the library and verify that reallly IS the issue.
Robert Ramey
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
participants (3)
-
Lars Viklund -
Nikolay Mladenov -
Robert Ramey