Game design, game programming and more

Your new() is not my new()

One of the problems I’ve experienced using third-party DLLs is the way that they handle new and delete for C++ classes, which can lead to memory leaks or even memory corruption. This was a particular problem when developing the Guild Wars model and animation exporter for 3ds Max, since 3ds Max uses plugins extensively, many written by different teams without the same coding standards as the original authors.

Here’s an example class API defined in a header file:

// FoozleDll.h
class CFoozle { ... };
CFoozle * MakeFoozle (CBarzle * bar);

And the associated CPP file:

// FoozleDll.cpp
CFoozle * MakeFoozle (CBarzle * bar) {
    return new CFoozle(bar);
}

In the application code we’re supposed to create and later delete this object, so here goes:

// Main.cpp
void ProcessData (CBarzle * bar, iostream * outfile) {
    CFoozle * foo = MakeFoozle(bar);        
    foo->WriteResults(outfile);
    delete foo;
}

That all looks pretty straightforward, but what isn’t immediately obvious is that the call to new() is made in the context of the Foozle DLL file, and the delete() call is done in the context of the application, which might be using an entirely different memory manager.

Since the DLL was compiled separately, it might link to the release version of the C runtime library, where new() calls malloc() behind the scenes. But the call to delete() occurs in the application code, which could link to the Debug library, which calls _free_dbg() instead of free(). When the application releases the memory via the call to delete it is calling the wrong memory manager, which leads to problems like the inability of the application to coalesce adjacent free memory blocks (memory leakage) or random memory corruption.

The correct solution is that a module which allocates an object should also free the object:

// FoozleDll.h
class CFoozle {
public:
    ...
    
    // This function must not be implemented in the header or the
    // linker will build the code to call the application delete()
    // function instead of the library delete() function.
    void DeleteThis ();
    
private:
    ~CFoozle (); // private so it can only be called by DeleteThis();
};

And the implementation:

// FoozleDll.cpp
void CFoozle::DeleteThis () {
    delete this;  // called from the right "delete context"
}

By calling delete() from within the same compilation unit, we can ensure that the compiler will generate a call to the correct delete function.

Incidentally, this same type of problem can occur in C, where a DLL function returns (for example), the result of strdup(), and the application is expected to call free() on the resulting string.

C++: so powerful, so easy to break things.

About Patrick Wyatt

As a game developer with more than 22 years in the industry I have helped build small companies into big ones (VP of Blizzard, Founder of ArenaNet, COO of En Masse Entertainment); lead the design and development efforts for best-selling game series (Warcraft, Diablo, Starcraft, Guild Wars); written code for virtually every aspect of game development (networking, graphics, AI, pathing, sound, tools, installers, servers, databases, ecommerce, analytics, crypto, dev-ops, etc.); designed many aspects of the games I've shipped; run platform services teams (datacenter operations, customer support, billing/accounts, security, analytics); and developed state-of-the-art technologies required to compete in the AAA+ game publishing business.

Comments

  1. Josh Wittner says

    Ref counting seems like a fairly straightforward/lightweight API contract that could reduce the amount of DeleteThis functions you’d have to write as a third party software provider.

  2. Patrick Wyatt says

    In the case of the code written for the article, 3dsMax defines the plugin API, which makes it a requirement to implement the DeleteThis function; reference counting isn’t possible in this case.

    But let’s imagine that you wrote a modeling application with your own plugin API that used reference counting instead of DeleteThis. Plugin creators writing plugins for your application (like me) would face the same problem documented in the article. The DecRef function would need to be implemented by the plugin creator (me) to ensure that the delete function occurred in the context of my plugin.

    And just as importantly, you’d have to be careful to ensure that all of your DecRef functions were implemented in C++ files (not header files) so that they would never get inlined into my plugin, or my plugin would call the wrong delete function for your objects when I called DecRef on them.

    This is why C++ is hard, and in fact why I’ve been coding in Scheme recently!

    • Note that when there’s a reference counting mechanism, there’s typically a factory that creates objects. In this case, 3dsMax only had to call your plugin’s factory function. Your plugin would certainly be compiled with the new operation in the same context as your DecRef’s delete operation.

      Sounds a lot like COM, sounds a lot like reinventing the wheel.

      Memory management has always been a module-contained operation. Using Scheme with any foreign function interface will make you think, on top of this, about what objects are pinned as to not move with garbage collection while calling foreign functions, if you should use C-like data structures or if your plugin lib knows your Scheme implementation’s tagged pointer/double details, and what not.

  3. Josh Wittner says

    I was imagining the RefCounter type would be provided for each plugin (and implemented in the plugin DLL). So the CFoozle classes would use the CFoozleRefCounter as a base. The only attempted gain was to reduce writing DeleteThis functions on all of the classes that the CFoozle DLL might provide. A CFoozleBase with virtual destructor and an implementation of DeleteThis would provide that same consolidation in the Max case.

  4. Patrick Wyatt says

    You’ve raised some interesting points. The key issue that always comes back to bite plugin writers is that the *location* (header or CPP file) of the code that calls “delete object” determines whether that code is correct.

    Here’s some code that will delete an object created by the plugin:

      void CSomeClass::DecRef () {
        if (!--m_refCount)
          delete this;
      }
    

    If this code is declared in a CPP file that is compiled into the plugin then everything works properly. If this code is declared in a header file included by users of the plugin then there is no guarantee that the right memory manager will be used to free the memory, and badness ensues (memory leaks, memory corruption followed by hard-to-trace crash bugs).

    This is how “delete” is implemented in C++ (when “compiled” to C):

      // C++ "delete object" in C:
      object->~CSomeClass();
      free(object);
    

    The problem is not the definition of the destructor, it is the definition of free.
    In the debug versions of the C runtime library free is overridden in crtdbg.h like this:

    #define free(p) _free_dbg(p, _NORMAL_BLOCK)
    

    So the plugin allocates memory using malloc from msvcrt.lib, and some other code later frees that memory with free from msvcrtd.lib, or another library like libcmtd.lib.

    I hope that makes it a bit clearer; it was a pain to figure out all this stuff (back in ~2002) when working on the 3dsMax model exporter for Guild Wars.

  5. Let’s note that for MSVC/Windows, this — MakeObj{new}, delete — will always work as long as the d’tor is implemented in the cpp file. If I’m not really mixing things up, the VC++ compiler will always generate the memory-cleanup-call at the destructor function site, so that the normal d’tor essentially becomes the DeleteThis() function. Of course this is highly non-portable and also quite tricky to detect statically if it’s OK.

  6. Graeme Wicksted says

    This comes up a lot on windows with different vc runtimes. Have you found it beneficial to create a wrapper using identical compiler/debug settings? Or is there a sneaky Patrick link-time app that can locate the correct free() by analyzing the machine code of the library ? ;) jk but that would be pretty bad—.

Speak Your Mind

*