Nathan Whitehorn wrote:
On 02/07/13 10:36, Robert Ramey wrote:
Nathan Whitehorn wrote:
What I'm concerned about is a situation like this:
Main library: Header: template<typename T> class I3Vector : public std::vector<T>, public I3FrameObject (our base class) { private: serialize(); };
BOOST_CLASS_EXPORT_KEY(I3Vector<T>) for a variety of T
Implementation:
Second library (a reasonably standard part of the software): Header: BOOST_CLASS_EXPORT_KEY(I3Vector<A>) for some other type A
Implementation:
The above looks good to me.
Third library (written by someone else as an addon): Implementation: Serialize an I3Vector<A> *without* including the second library's header
Hmmm - so library three includes it's own BOOST_CLASS_EXPORT_IMPL(I3Vector<A>) ? as a static library - no problem. as a dll - extra guid records created. Probably not a problem. If library three doesn't include BOOST_CLASS_EXPORT_IMPL(I3Vector<A>) it would likely use the one from from library two which depending on cirucumstances could produce unexpected results. <aside> At one point I included code to trap multiple guid records of the same type. The code is still in there - which a big comment. This enforced the ODR regarding guid types. But I had to comment it out because it required users to felt it was too difficult to limit instantiations and depended upon things "Just working" even with multiple instantiations. </aside>
What happens here is that library #3 will usually appear to work -- and certainly compile and link without issue -- because all the serialization instantiation was done in library #2. *However*, if library 3 is loaded before library 2, both library 3 *and* 2's attempt to serialize I3Vector<A> will starting failing with an unregistered class exception.
I'm not really sure about that. the guid table permits multiple records. (Though I think it shouldn't - see aside above). So I would expect that library 3 would work if library 2 was already loaded and fail if it wasn't - again - nasty behavior to try to resolve)
This is because extended_type_info_typeid<A> is a singleton and the first instance of it (as well as the now competing definitions of the classes if not fully inlined) came from library 3 where the GUID template specialization had not happened.
extended_type_info_typeid<A> is a singleton - BUT the concept of singleton in the presence of dynamically loaded DLLS is another surprise! Turns out that there is a separate table of guids for each DLL. Since static variables are created at the module level - not at the program level. This is why one can dynamically unload a DLL with static variables in it. To summarize DLL or Mainline is called a module for each Module there is one static table of guids for each instatiation in each module a guid record is added to the appropriate static table when a record is looked up - it's looked up in the guid table which corresponds to the module where the look up is invoked. and it's not always obvious where the looking up is happening class base - in a dll class derived : public base // in some other module. etc. And of course just when you've got it all figured out - some one raises the issue of multi-threading and thread safety in the presence of dynamically loaded DLLS. (hint, unload DLLS in reverse order of loading them - another thing to keep track of). Note that the standard says nothing - as far as I can tell - about the existence, lifetime, location, etc. of DLLs. In fact, it says nothing at all about DLLS so one has to sort of guess what all the compilers do. This isn't as bad as it sounds as requirements more or less dictate how things have to work - but it takes a while to come to that conclusion.
This is an awful problem to have to debug, especially when libraries 2 and 3 are written by unrelated parties and the behavior happens to depend on the user's choice of load order (the libraries come in through RTLD at runtime). It means that all possible I3Vector<T> need to have keys exported in a common header somewhere that you can't possibly avoid including if you ever use an I3Vector<T>. Otherwise, everything can break everywhere if you happen not to have included that header in wherever happens to be the first occurrence of the type from the perspective of the (potentially dynamic) linker.
I'm sympathetic. But I don't know what I can do to help. Basically I think it's a problem bigger than serialization - it just happens to come up here more frequently. Since it's infrequent one is never really prepared for it. I would step back and consider the more generall problem. If I ship a DLL which includes an implementation of I3Vector<A> then I should find a way to prohibit/discourage users from re-defiining this. If a user does this, there is a risk of accidently using part of the implemention from library 2 and library 3 together without knowing it. Maybe it's a question of puting library 2 in a private namespace or having a "trapping header" with declares all the library 2 interface with static assert if something is re-instantiated. I don't see a magic solution - just trick work. Of course this is why we earn the big bucks. <aside> Note this has already happened to utf-code-cvt. Due to some inconsistencies in library function signatures accross standard library implementations, in some cases code will impord from the standard library functions which are defined inside of utf8-codecvt -a boost library. This manifests itself as a tesst failure in the serialization library. This took a while to find. </aside>
What I was hoping to do is to replace (still before main is called) any possible NULL GUIDs for a class with a non-NULL one if the relevant extended_type_info ever gets instantiated with a non-NULL GUID (which would require some changes to how the extended_type_info_typeid constructor works, but that's a separate issue). The idea would be that the GUID attached to a BOOST_CLASS_EXPORT(), when it runs, would be the final word on the matter instead of a leftover NULL from an instantiation in some place that didn't know about the GUID.
I don't think we create a NULL guid - I think we just fail to create one. (forgive me if I'm mis-remembering). I understand the appeal trying to make the code more clever to address the situation. But I think the code is already on the verge of being so clever that it's in danger of being too hard to understand how to use. (much less make work and verify that it does!) To me this is more hiding the source of the problem (violation of ODR in DLLS) rather than fixing it (multiple instantiations). Basically I think the best long term solution is to bite the bullet and restructure the code somewhat to avoid the problem. One thing that might be useful is to re-enable or make optional the trap which throws an exception when the ODR is violated at runtime when the DLLS are loaded. I remember looking into making this and option - but I think it was too deep to make it easily accessible. And besides - most people who encountered the trap considered it a bug in the library. Though it might surprise some on this list, I can't fight every battle so I just commented it out and moved on.
Thanks for the suggestions. They mostly reflect what we were already doing -- people get beaten with a stick if they try to instantiate serialize methods in header files or multiple times and they are usually kept in implementation files for that reason.
It's mostly a question of scale. For a smaller simple program inline is just fine and very simple and convenient. But once one moves into dynamically loaded/unloaded code there are whole new set of issues to consider - and they are not obvious.
We've mostly given up on automatic instantiation, but kept it easy to extend if you want to (see the problem with I3Vector above) -- although it would be amazing if you figured out how to do it.
lol - When I set the goal of building a system which would permit non-intrusive serialization (ie. no special base class), I totally mis-underestimated how difficult it would be create such a system. Looking back at what we've created - actually, I'm personally amazed how far we've been able to come towards a system which mostly "just works". Robert Ramey
-Nathan