nlohmann / json

JSON for Modern C++
https://json.nlohmann.me
MIT License
42.18k stars 6.65k forks source link

Memory leak detection with basic usage of NLOHMANN_JSON_SERIALIZE_ENUM #3939

Closed pierre-quelin closed 1 year ago

pierre-quelin commented 1 year ago

Description

The basic usage of NLOHMANN_JSON_SERIALIZE_ENUM seems to return an error with _CrtMemCheckpoint Microsoft tool.

Reproduction steps

A basic google test return "Memory leak of 108 byte(s) detected" with the code provided as example.

Expected vs. actual results

Expected no memory leaks vs 108 byte(s).

Minimal code example

// ================================
// example enum type declaration
enum TaskState {
    TS_STOPPED,
    TS_RUNNING,
    TS_COMPLETED,
    TS_INVALID = -1,
};

// map TaskState values to JSON as strings
NLOHMANN_JSON_SERIALIZE_ENUM(TaskState, {
    {TS_INVALID, nullptr},
    {TS_STOPPED, "stopped"},
    {TS_RUNNING, "running"},
    {TS_COMPLETED, "completed"},
    })

TEST(TestGensBoard, MemoryLeak)
{
    test_helpers::MemoryLeakDetector detector;

     // json string to enum
     json j3 = "running";
     assert(j3.get<TaskState>() == TS_RUNNING);
}

// ================================
MemoryLeakDetector::MemoryLeakDetector()
{
    _CrtMemCheckpoint(&_memState);
}

MemoryLeakDetector::~MemoryLeakDetector()
{
    _CrtMemState stateNow, stateDiff;
    _CrtMemCheckpoint(&stateNow);
    const auto diffResult = _CrtMemDifference(&stateDiff, &_memState, &stateNow);
    if (diffResult)
        reportFailure(stateDiff.lSizes[1]);
}

void MemoryLeakDetector::reportFailure(unsigned int unfreedBytes)
{
    FAIL() << "Memory leak of " << unfreedBytes << " byte(s) detected.";
}

Error messages

No response

Compiler and operating system

Microsoft Visual C++ 2019 (Win32), Window10

Library version

v3.11.2

Validation

gregmarr commented 1 year ago

I would recommend using void _CrtMemDumpAllObjectsSince(const _CrtMemState *state); to report more information about the leak.

pierre-quelin commented 1 year ago

0 bytes in 0 Free Blocks. 108 bytes in 6 Normal Blocks. 0 bytes in 0 CRT Blocks. 0 bytes in 0 Ignore Blocks. 0 bytes in 0 Client Blocks. Largest number used: 0 bytes. Total allocations: 144 bytes. Dumping objects -> {2239} normal block at 0x0154C258, 8 bytes long. Data: <( T > 28 E0 54 01 00 00 00 00 {2238} normal block at 0x0154E028, 28 bytes long. Data: 58 C2 54 01 63 6F 6D 70 6C 65 74 65 64 00 CD CD {2237} normal block at 0x0154C098, 8 bytes long. Data: < T > C8 E5 54 01 00 00 00 00 {2236} normal block at 0x0154E5C8, 28 bytes long. Data: < T running > 98 C0 54 01 72 75 6E 6E 69 6E 67 00 CD CD CD CD {2235} normal block at 0x0154C060, 8 bytes long. Data: < T > 00 E1 54 01 00 00 00 00 {2234} normal block at 0x0154E100, 28 bytes long. Data: <` T stopped > 60 C0 54 01 73 74 6F 70 70 65 64 00 CD CD CD CD Object dump complete.

gregmarr commented 1 year ago

That looks to me like it's from the testing infrastructure. If you remove the json variable, do you get the same result?

gregmarr commented 1 year ago

Ah, it's probably from the static storage inside those functions that gets initialized on the first use:

static const std::pair<ENUM_TYPE, BasicJsonType> m[] = __VA_ARGS__;

If that's the case, then this will also result in the exact same 108 byte leak:

test_helpers::MemoryLeakDetector detector;

 // json string to enum
 json j3 = "running";
 assert(j3.get<TaskState>() == TS_RUNNING);
 json j4 = "running";
 assert(j4.get<TaskState>() == TS_RUNNING);
 json j5 = "running";
 assert(j5.get<TaskState>() == TS_RUNNING);

and this will result in no leak:

 json j3 = "running";
 assert(j3.get<TaskState>() == TS_RUNNING);

test_helpers::MemoryLeakDetector detector;

 json j4 = "running";
 assert(j4.get<TaskState>() == TS_RUNNING);

So this is only a "leak" because the static variables are initialized after the MemoryLeakDetector and not destroyed until after the detector.

pierre-quelin commented 1 year ago

Thank you. I still have to see how to correct my unit test, especially for the user classes.