Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Memory Leak detected for tf::Taskflow v3.3.0 onwards (False Positive) #1701

Open
coder137 opened this issue Dec 3, 2022 · 6 comments
Open

Comments

@coder137
Copy link

coder137 commented Dec 3, 2022

Cross-posting from here: taskflow/taskflow#450

Test setup being used

#include "taskflow/taskflow.hpp"

// NOTE, Make sure all these includes are AFTER the system and header includes
#include "CppUTest/CommandLineTestRunner.h"
#include "CppUTest/MemoryLeakDetectorNewMacros.h"
#include "CppUTest/TestHarness.h"
#include "CppUTest/Utest.h"

TEST_GROUP(TaskStateTestGroup)
{
};

TEST(TaskStateTestGroup, TestTask) {
  tf::Taskflow taskflow;
  taskflow.emplace([]() { UT_PRINT("Hello World"); });

  // tf::Executor executor(1);
  // executor.run(taskflow).wait();
}

int main(int ac, char **av) {
  return CommandLineTestRunner::RunAllTests(ac, av);
}

Error Output

nnaidu@LAPTOP-S0KH4M5U:~/repository/build_in_cpp/build$ cmake --build . -j12 && ctest --output-on-failure -C Debug -R test_task_state
ninja: no work to do.
Test project /home/nnaidu/repository/build_in_cpp/build
    Start 5: test_task_state
1/1 Test #5: test_task_state ..................***Failed    0.00 sec

../buildcc/lib/env/test/test_task_state.cpp:21: error: Failure in TEST(TaskStateTestGroup, TestTask)
        Memory leak(s) found.
Alloc num (7) Leak size: 65584 Allocated at: <unknown> and line: 0. Type: "new"
        Memory: <0x56171fe78f90> Content:
    0000: 78 85 e7 1f 17 56 00 00  a0 85 e7 1f 17 56 00 00 |x....V.......V..|
    0010: a0 85 e7 1f 17 56 00 00  01 00 00 00 00 00 00 00 |.....V..........|
    0020: 00 00 00 00 00 00 00 00  c0 8f e7 1f 17 56 00 00 |.............V..|
    0030: 00 00 00 00 00 00 00 00  d8 8f e7 1f 17 56 00 00 |.............V..|
    0040: 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00 |................|
    0050: 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00 |................|
    0060: 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00 |................|
    0070: 00 00 00 00 00 00 00 00  58 ec 77 1e 17 56 00 00 |........X.w..V..|
    0080: 48 eb 77 1e 17 56 00 00  00 00 00 00 00 00 00 00 |H.w..V..........|
    0090: 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00 |................|
    00a0: ff 00 00 00 00 00 00 00  50 90 e7 1f 17 56 00 00 |........P....V..|
    00b0: 50 90 e7 1f 17 56 00 00  60 90 e7 1f 17 56 00 00 |P....V..`....V..|
    00c0: 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00 |................|
    00d0: 78 90 e7 1f 17 56 00 00  78 90 e7 1f 17 56 00 00 |x....V..x....V..|
    00e0: 88 90 e7 1f 17 56 00 00  00 00 00 00 00 00 00 00 |.....V..........|
    00f0: 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00 |................|
    0100: 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00 |................|
    0110: 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00 |................|
    0120: 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00 |................|
    0130: 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00 |................|
    0140: 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00 |................|
    0150: 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00 |................|
    0160: 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00 |................|
    0170: 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00 |................|
    0180: 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00 |................|
    0190: 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00 |................|
    01a0: 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00 |................|
    01b0: 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00 |................|
    01c0: 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00 |................|
    01d0: 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00 |................|
    01e0: 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00 |................|
    01f0: 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00 |................|
    0200: 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00 |................|
    0210: 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00 |................|
    0220: 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00 |................|
    0230: 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00 |................|
    0240: 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00 |................|
    0250: 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00 |................|
    0260: 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00 |................|
    0270: 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00 |................|
    0280: 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00 |................|
    0290: 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00 |................|
    02a0: 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00 |................|
    02b0: 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00 |................|
    02c0: 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00 |................|
    02d0: 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00 |.............
etc etc etc etc. !!!! Too many memory leaks to report. Bailing out
Total number of leaks:  1


.
Errors (1 failures, 1 tests, 1 ran, 0 checks, 0 ignored, 0 filtered out, 0 ms)



0% tests passed, 1 tests failed out of 1

Total Test time (real) =   0.01 sec

The following tests FAILED:
          5 - test_task_state (Failed)
Errors while running CTest
  1. What could be the potential reasons for a memory leak to be detected?
  2. Comparison made
  • Simple Taskflow only example run with Valgrind (No memory leak found)
  • The above example (Taskflow + CppUTest run with Valgrind)

Memory leak is detected

../buildcc/lib/env/test/test_task_state.cpp:23: error: Failure in TEST(TaskStateTestGroup, TestTask)
        Memory leak(s) found.
Alloc num (3) Leak size: 8 Allocated at: ../buildcc/lib/env/test/test_task_state.cpp and line: 23. Type: "new"
        Memory: <0x4dabcf0> Content:
    0000: 78 aa 15 00 00 00 00 00                          |x.......|
Total number of leaks:  1


.
Errors (1 failures, 1 tests, 1 ran, 0 checks, 0 ignored, 0 filtered out, 98 ms)

Other information

  • CppUTest versions tested v4.0 and latest-passing-build
  • tf::Taskflow works till v3.2.0, v3.3.0 onwards has this memory leak detected.
    According to the thread there was a race condition that was eliminated as well as other thread_local static variables removed
  • Testing the code on Windows 10 WSL Ubuntu 20.04
@coder137 coder137 changed the title False Memory Leak detected for tf::Taskflow v3.3.0 onwards Memory Leak detected for tf::Taskflow v3.3.0 onwards (False Positive) Dec 3, 2022
@basvodde
Copy link
Member

basvodde commented Dec 5, 2022

Most likely this involves a static variable somewhere that is allocated during the test and not freed at the end.

If you want to figure out what it is, I recommend calling crash_on_allocation_number(7) in your test. This will crash CppUTest on the 7th allocation (which according to the error message was the leak). You can then run it in a debugger and check the stack trace, which will point you to the place where the memory is allocated (and probably to the static).

Good luck!

@coder137
Copy link
Author

coder137 commented Dec 8, 2022

Thanks for the answer, I was able to find the global/static variable that was causing the problem.

Will update my analysis here for those that have a similar problem with CppUTest and Taskflow

File: object_pool.hpp

template <typename T, size_t S>
template <typename... ArgsT>
T* ObjectPool<T, S>::animate(ArgsT&&... args) {

When creating a new block

    // create a new block
    else {
      //printf("create a new superblock\n");
      _gheap.mutex.unlock();
      f = 0;
      //s = static_cast<Block*>(std::malloc(sizeof(Block)));
      s = new Block(); // This is flagged as a leak

The earlier implementation (before v3.3.0), used malloc instead of new which is why the CppUTest memory leak detector did not flag this as a leak.

The ObjectPool is instantiated globally in the header file like this
File: graph.hpp

inline ObjectPool<Node> node_pool;

@coder137
Copy link
Author

coder137 commented Dec 8, 2022

@basvodde Since this global variable is causing a problem are there workarounds to this?

I do not want to completely turn off the Memory Leak detector plugin in CppUTest and my project has a hard dependency on Taskflow.

Any help would be appreciated.
Thanks

@basvodde
Copy link
Member

basvodde commented Dec 9, 2022

Yes, there is a standard workaround for this, actually two.

If it is possible, then you can release the static within the test. So... in your case... is there a way to release that Block via some function call such as "clearAllCachedBlocks". When you test-drive with CppUTest then typically you always create such a function to make sure that the tests also releases global/static data.

If that is not possible (and I suspect that this is the case) then the common workaround is to try to allocate that static in the main. The memory leak detector works per test, so when the static is allocated before the test starts then it will not flag it as a leak. So, it is not uncommon to have a main that creates a bunch of objects to get all the uncontrollable statics/globals initialized and then it starts the tests.

If you can, option 1 is best. If you must, option 2 is almost always possible.

@tsung-wei-huang
Copy link

tsung-wei-huang commented Dec 11, 2022

@basvodde (copying @coder137 in the loop),

Would you please tell me why using 'new' has a different leak result from 'malloc' between the versions?

@basvodde
Copy link
Member

@tsung-wei-huang Sure.

By default, in CppuTest, the operator new is overloaded to keep track of memory allocated so that it can discover memory leaks. The malloc is not overloaded by default as there is no C language support for overloading that. It is still possible through a #define which replaces malloc with cpputest_malloc... but then that only works when you compile all of your code with that #define enabled. (this is possible by adding --include MemoryLeakDetectorMalloc.h to the compiler options of your product).

Thus by switching from malloc to switch, it enabled CppUTest to discover the memory allocation, keep track of it, and it discovered it was not released within the test where it was allocated (because of the static).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants