Problems with memory using msdfgen DLL with Staticly linked Runtime Library (/MT)

[Erfan-Ahmadi]

We're using msdfgen-core in our project. When building it as a dll (msdfgen-cored.dll) we mistakenly used (/MT) flag which uses the static version of the runtime library in the msdfgen dll; however that led me to find some issues with msdfgen I wanted to share:

If our application (exe) or some other dll is calling msdfgen function it might call it with a different version of the runtime library and this mismatch of C++ runtime libraries can be real problematic (for example changes to std::vector implementation and how it works with memory, esp with usages of vectors in msdfgen code).

This means that our allocations/deallocation, construction/deconstruction, new/deletes must all happen in a single module to avoid working with pointers/vector/memory allocated by other module which can be bug prune. (debug_heap asserts in runtime helps us avoid it)

With all that in mind here are the issues:

1. msdfgen classes have constructors implemented in cpp (eventually in dll) but the destructors are implicit in header for example Shape.h if used through a /MT built dll, it will construct the std::vector<Contour> contours; using it's runtime and it will destruct it in the caller module in ~Shape

This happens because we're exporting using Module Definition Files (.def) which doesn't export classes default destructors and we're not using dll_export/import on the whole class. using class MSDFGEN_PUBLIC Shape { on the whole class is a fix. Alternatively: implement deconstructors explicitly in cpp and add to .def

2. inconsistency and unclear interface: in EdgeHolder::16 we can pass in any edge segment pointer that we have "new"ed on the caller module but the delete is happening inside msdfgen dll in EdgeHolder::~EdgeHolder. This will cause confusion to the user to attemp creating Linear/Quad/Cubic segments and pass by pointer instead of using EdgeSegment::create functions.

Suggestion: make this trivial constructor protected/private to protect against incorrect usage

Let me know about your mindset on this and whether you'd like me to open a PR to fix these.

Additional Resources:

• https://learn.microsoft.com/en-us/cpp/build/reference/md-mt-ld-use-run-time-library?view=msvc-170

[Chlumsky]

It seems to me that the only issue here is mixing different runtimes. I don't believe it's possible to eliminate that issue completely without making the library header-only. For example, even if the entire Shape class had the DLL specifier, the moment you accessed its contours vector from your module, it would also use the wrong vector version.

[Erfan-Ahmadi]

Yes that's the issue.

You don't need the library to be header only. You just need to make your vector or std containers private and provide accessors and modifiers.

Which is why I don't understand why there is addContour or addXXX but the container is also public to user to be able to modify and push back into.

The rule of thumb for me when designing an API is to never expose std containers, but pointers and sizes, or accessors and modifiers.

But almost always، mixing runtimes is not a good idea at all. Which is why I suggest you put a warning in your readme at least.

[Chlumsky]

First of all, the "architecture" of the library was designed 10 years ago and I couldn't have predicted its popularity. I also care about backwards compatibility, so I can't just arbitrarily change the classes API.

Personally, I don't think you can mix runtimes when dealing with C++ libraries in general and no amount of sanitizing these classes will change that. The only exception to that is when you have strictly C DLL bindings. Therefore, I believe it would be futile to try to address this in my code.