Menu

Static Initialisation Order Fiasco

Sc4Freak
2008-03-30
2013-04-17
  • Sc4Freak

    Sc4Freak - 2008-03-30

    The issue arises in the FCollada API. I draw your attention to FMAllocator.cpp, and FArchiveXML.h.

    In FMAllocator.cpp are two globals: af and ff. These two globals are function pointers that control memory allocation. They are given the default values of malloc and free. Since they're globals, they're constructed before the program starts.

    Now take a look at FArchiveXML.h, line 44, there's a static tree object named "xmlLoadFuncs". Since it's static, it's also constructed before the program starts. The problem is that the constructor of this object allocates some memory. Using - you guessed it - the memory allocation functions in FMAllocator.cpp.

    This is the static intitialistion order fiasco - you've got a 50% chance of the program randomly crashing even before main() is reached, depending on how the compiler is feeling that day. If the globals in FMAllocator.cpp are constructed first, all's fine and dandy. But if the tree in FArchiveXML.h is constructed first, then it tries to access the af global in FArchiveXML.h which hasn't been initialised yet!

    The result is an access violation caused by the constructor of the static tree object in FArchiveXML.h trying to call an invalid function pointer (af). Since there is no defined order of intialisation, this is a completely random crash and it's been because of pure luck that nobody's encountered it yet.

    The solution to this problem is relatively simple. Wrap the globals in a function. This should resolve the problem in this particular case. However, I'm not completely familiar with the codebase, so many more cases like this may still exist.

    // ----------------------------------------------------------------------
    // FArchiveXML.h

    #include "StdAfx.h"
    #include "FMAllocator.h"

    namespace fm
    {
        // default to something: static initialization!

        AllocateFunc& af()
        {
            static AllocateFunc* ret = new AllocateFunc(malloc);
            return *ret;
        }
        FreeFunc& ff()
        {
            static FreeFunc* ret = new FreeFunc(free);
            return *ret;
        }
        //AllocateFunc af = malloc;
        //FreeFunc ff = free;
       
        void SetAllocationFunctions(AllocateFunc a, FreeFunc f)
        {
            af() = a;
            ff() = f;
        }

        // These two are simple enough, but have the advantage of
        // always allocating/releasing memory from the same heap.
        void* Allocate(size_t byteCount)
        {
            return (*af())(byteCount);
        }

        void Release(void* buffer)
        {
            (*ff())(buffer);
        }
    };

     
    • Jon Watte

      Jon Watte - 2008-04-08

      However, if there are other static constructors that allocate memory, then the functionality of SetAlloicationFunctions() is basically useless, because you will break whatever was allocated before that function. Thus, that function should be removed.

      Also, instead of using "new AllocateFunc," the function-static members should probably just be direct instances that don't go through operator new and delete. The runtime will ensure that those are constructed the first time the functions are entered, even during initialization.

      Hence, I'd remove SetAllocationFunctions() from the file, and then provide the following implementation:

      AllocateFunc& af()
      {
        static AllocateFunc ret(malloc);
        return ret;
      }

      FreeFunc& ff()
      {
        static FreeFunc ret(free);
        return *ret;
      }

       

Log in to post a comment.