Undefined order of calling dtors may cause segfault

[mikhirev]

Hello!

I'm in doubt should I report this issue against libzypp or zypper, but it was introduced by this commit in libzypp.

The problem is that there are the _theGlobalLock static object in libzypp/zypp/ZYppFactory.cc and the God global object in zypper/src/Zypper.cc. The order of their initialisation is undefined, and that's OK. But the order of calling their dtors is also undefined. If God is destroyed first, everything is fine, but if _theGlobalLock is destroyed first, dtor of God tries to call _theGlobalLock.reset() that is already unavailable.

As this issue is caused by undefined behavior, it is hard to reproduce. To be honest, I don't know, can it be reproduced on GNU systems or not, but we have zypper ported to FreeBSD (with numerous modifications to make it possible), and it always segfaults after exit().

[mlandres]

We'll think about a solution. Construction of God mus assert _theGlobalLock is completely constructed before it ends. (Fix may take a bit time as urgent tasks and vacations are pending)

[mikhirev]

The possible solution is to make _theGlobalLock a member of ZYppFactory instead of keeping it global. I tried this and it seems to work, however it requires huge modifications and breaks binary compatibility because getZYpp() becomes non-const. If you think this is a correct way, I can open a PR.

[mlandres]

I suppose avoid breaking binary compatibility was the reason for doing it the way it currently is.

My naive approach would have been to replace the construction of God in zypper by something like

inline std::nullptr_t dtorHack()
{
  ZYppFactory::instance(); // assert statics in ZYppFactory.cc are constructed
  return nullptr;
}
ZYpp::Ptr God( dtorHack() );

The call into the ZYppFactory translation unit should make sure, _theGlobalLock there is fully constructed before God. Due to this God should be destructed first.

Would this work for you as well?

[mikhirev]

Sorry, I don't understand how can this help. The order of static objects initialization is still undefined, calling ZYppFactory::instance() changes nothing here. If it could change the initialization order, there wouldn't be such a notorious thing as "static initialization order fiasco".

[mlandres]

Switching the statics in ZYppFactory.cc to construct on first use, the ZYppFactory.cc ctor is able to enforce their construction.

[mikhirev]

Calling a ctor from a translation unit will initialize all statics in that unit? Why?

[mlandres]

(Sorry for my bad english) Not out of the box of course. The static vars in ZYppFactory.cc must be moved into static functions. The ctor (or some other static method we install there) will be able to call this function to force the initialization before dtorHack returns.

[mikhirev]

Well, this should work, but I don't see a possibility to make _theGlobalLock local to some function. And such a solution would be very hackish…