Memory leaks just by including doctest.h

[edmundvermeulen]

Description

We upgraded from doctest 1.2.6 to 2.2.3. Now Visual Studio reports memory leaks.

Steps to reproduce

The following simple example code gives 3 memory leaks when run.

#include "stdafx.h"

#define DOCTEST_CONFIG_IMPLEMENT
#include "doctest.h"

int main()
{
    int flag = _CrtSetDbgFlag(_CRTDBG_REPORT_FLAG);
    _CrtSetDbgFlag(flag | _CRTDBG_ALLOC_MEM_DF | _CRTDBG_LEAK_CHECK_DF);

    return 0;
}

Detected memory leaks!
Dumping objects ->
{175} normal block at 0x004F5620, 8 bytes long.
 Data: <  O     > EC 98 4F 00 00 00 00 00 
{163} normal block at 0x004F5578, 8 bytes long.
 Data: <| O     > 7C 8E 4F 00 00 00 00 00 
{156} normal block at 0x004F57E0, 8 bytes long.
 Data: < 5O     > AC 35 4F 00 00 00 00 00 
Object dump complete.

Extra information

• doctest version: 2.2.3
• Operating System: Windows 10
• Compiler+version: Microsoft Visual Studio 2015

[onqtam]

strange... valgrind on Unix doesn't report anything.

Version 2.0+ uses a lot more things from the STL - this could be a bug in the toolchain/stl (i've had such issues many times before where a specific version of the compiler is causing trouble).

I'll try to reproduce it and investigate but I don't know when I'll get to this.

[edmundvermeulen]

The same leaks occur in VS2019 Preview 2.

Further investigation shows that this is caused by g_infoContexts being thread_local. When DOCTEST_THREAD_LOCAL is defined to nothing then the leaks go away. Is it necessary to have it be thread_local if all the tests run from the same thread?

[onqtam]

All tests always run from the same thread but if one test case spawns multiple threads and uses asserts and logging with INFO() - then that needs to be thread-local. If you don't have such a scenario (a test case spawning multiple threads which use doctest asserts) you may indeed use DOCTEST_THREAD_LOCAL and define it to nothing.

Thanks for diagnosing this - I'm very curious as to why a thread-local global std::vector is a problem. If you have any ideas I would be grateful - otherwise I'll look into it when I have the chance... But for now version 2.3 with xml reporting is the priority.

[edmundvermeulen]

Solution: creating the static thread_local object from a getter function solves it. Like this:

    std::vector<IContextScope*>& getInfoContexts() {
        static DOCTEST_THREAD_LOCAL std::vector<IContextScope*> g_infoContexts; // for logging with INFO()
        return g_infoContexts;
    }

[onqtam]

Is this solution related to the "static initialization order ‘fiasco"? This global is used only after main() is entered so all globals (including this one) should be already initialized... Thanks for letting me know it works this way! I'm still really puzzled why this is necessary though...

If I don't come up with anything else as a solution I might add this ... "fix" with a fat TODO next to it for the next version.

[edmundvermeulen]

Perhaps. It is a bit of a bad experience to see memory leaks just by including the header, though.

[onqtam]

So... Could you do this fix locally for your copy of doctest for now, and wait for me to try to figure this thing out when I have the time?

[edmundvermeulen]

Yes. I have already fixed it here locally.

[onqtam]

So I just tried copy-pasting your example and am unable to reproduce this with VS 2017.

Are you running the executable in some special way? I haven't investigated what _CrtSetDbgFlag and those flags do but I assume the program would check itself upon exiting...? I tried the Debug config since in Release that call for setting flags is a no-op.

How did you diagnose that the problem was the thread-local global? There are other thread-local global variables apart from it...

What else do you have in stdafx.h? Is it the default auto-generated one by VS?

If anyone else can contribute information to this issue I would be grateful.

[edmundvermeulen]

This is with a clean new Console Application project created via the project wizard.

Don't you see the memory leaks in Visual Studio's output window?

[edmundvermeulen]

To be clear: The memory leaks are not printed with the console output, but rather in the Output window inside Visual Studio itself.

[onqtam]

I previously tried it in a solution generated with CMake but now just tried it with a new Console Application project created from the studio - and got no errors (both x86 and x64). I used VS 2019 Preview 4.3.

Notice how I don't have a stdafx.h generated - perhaps whatever is in there might be the problem.

I highly suspect that this has nothing to do with doctest... memory leaks are diagnosed on the CI with industry-standard tools such as valgrind and the sanitizers on Linux. And it is really strange that only that thread-local global is problematic.

I'm currently out of ideas about this and don't want to commit fixes which I don't understand. I just released version 2.3.0 but will keep this issue open if any new information comes up - for now I'm clueless :|

[onqtam]

Well I'm going to close this... It will remain for others to find and inspect if anyone else hits this problem, but as far as I understand there shouldn't be anything wrong with that thread local, and there are no problems with the other thread locals as well... I cannot reproduce this locally - I'll need even more information on how to reproduce it - with an even smaller example... perhaps using cl.exe on the command line with the full list of flags or something, and not calling the executable through the debugger maybe... I don't know. But the fix suggested here doesn't seem right to me. You could still define DOCTEST_THREAD_LOCAL to nothing locally.