GTA does not detect process termination triggered by CRT memory leak detection

[alfredskpoon]

We use CRT Debug Heap to detect memory leaks in our applications and unit tests as described here:

https://docs.microsoft.com/en-us/visualstudio/debugger/finding-memory-leaks-using-the-crt-library?view=vs-2017

This is also used to fail our Builds by terminating the process with exit code 1.

Unfortunately since migrating to Google Test using Google Test Adapter, I'm finding memory leaks no longer trigger build failures and I think Google Test Adapter does not reflect process exit code in test failures. Is there any way to trigger test/build failures from abnormal process termination on process exit?

I've attached 2 files, test_leak.cpp and test_leak_main.cpp containing sample code to reproduce the issue. When a memory leak is simulated in the test file, the leak is detected on process shutdown and the process exits with exit code 1.
test_leak.zip

[csoltenborn]

Thanks for filing the issue and for the sample code! Is it ok if I add that code to my SampleTests suite?

Now I better understand the approach, and I might indeed have a solution. Currently, GTA completely ignores the exit code of the test executables (except during test discovery, but this is merely to recognize if something has gone wrong with that). My idea (which I have already partly implemented) is as follows: I will provide a new option which allows to specify a test name (e.g., MemoryLeakTest). If such a test name is specified, GTA will generate an additional test for each test executable (named <executable>.MemoryLeakTest in our example), and that will pass if and only if the test executable's exit code is 0. Additionally, I will provide a token which - if found in the executable's output - will cause the following output to be added as error message to the test in case it fails.

For your code, this would mean that you would not return the result of RUN_ALL_TESTS() in your main method any more, but ignore it and just return 0. This way, your executable will only return 1 if there have been memory leaks, thus making the test (and therefore your build) fail.

The only drawback I see is that the test executable will not behave as "normal" gtest executables do (which produce an exit code of 0 if and only if no test has failed), but if the tests are only executed through GTA (be it locally or on the build server), that shouldn't be an issue. I would however probably add some output to the test executable to indicate that different behavior.

Would that solve your problem?

[alfredskpoon]

Sure, feel free to add the memory leak test code in your sample tests suite. There is nothing proprietary in the contents.

Thanks for explaining your proposal. Let me summarize to make sure I understand correctly:

• Add a new option in GTA, which if specified (for example MemoryLeakTest) will generate a new test
  o . MemoryLeakTest will only pass if executable exits with 0
• So if there are 3 tests in , the list of tests will look like this
  o .
  o .
  o .
  o . MemoryLeakTest
• No test code is actually run for MemoryLeakTest, it's just a placeholder to tag a non zero exit code
• In the test main program where RUN_ALL_TESTS() is called
  o Always return 0

This approach should work and should solve the issue of not detecting memory leak failure indicated by a non zero exit code. I just have a few questions,

1. Is there any harm (or any reason) why the test main should not continue to return the result of RUN_ALL_TESTS()? I mentioned this because with the change to always return 0 in test main, a failure in say will not propagate back as exit code 1 if the test executable is run standalone. (Running the test through GTA in VS Test Explorer or as a Build are not affected obviously)

2. Will the total list of tests executed include MemoryLeakTest?

Thanks!

[alfredskpoon]

The lists of tests didn't show up correctly, here's what I meant:

• executable.test1
• executable.test2
• executable.test3
• executable.MemoryLeakTest

[alfredskpoon]

Also, question 1 above did not show up correctly, it should be:

1. Is there any harm (or any reason) why the test main should not continue to return the result of RUN_ALL_TESTS()? I mentioned this because with the change to always return 0 in test main, a failure in say executable.test2 will not propagate back as exit code 1 if the test executable is run standalone. (Running the test through GTA in VS Test Explorer or as a Build are not affected obviously)

[csoltenborn]

I'd seen the exit(1) statement and had assumed that only in that case, the executable would have exit code 1, and the result of RUN_ALL_TESTS() otherwise. In that case, if there would not be a memory leak, but one of the tests had failed, and the result of RUN_ALL_TESTS() would be propagated back as exit code, the added test would obviously still be flagged as failed. That's why i had suggested to always return 0. Why is this mechanism not in place when run standalone?

In any way, the point is that the added test would be green iff the executable returns 0 when run by GTA.

Concerning you questions: Your description is accurate. The added test will be treated as any other test (despite having no source location) and will thus be included in the total list of tests.

[alfredskpoon]

OK, if I understand correctly,

1. In case there is no memory leak, and one of the tests (say test2) failed, the added test (MemoryLeakTest) is flagged as failed.
2. The added test (MemoryLeakTest) is flagged as failed because the test executable exit code is 1
3. The test executable exit code is 1, presumably because it returned the status of RUN_ALL_TESTS() ?

Is this logic correct? If so, should the test main still be always returning 0?

When the test executable is running standalone (type test_program.exe in command prompt), GTA is not involved at all and so the test main return code from RUN_ALL_TESTS() is the only success/failure return code indication.

At our workplace we have 3 scenarios under which unit tests are run,

1. (For Developers) Visual Studio in Test Explorer (Using GTA)
2. (For Developers) Windows command prompt (No GTA) using a powershell scripts that runs all test executables in parallel
3. (For Builds) Run by TFS Build agent (using GTA)

So for scenario 2), the test executables are run without GTA, and so returning status of RUN_ALL_TESTS() I think is necessary.

Also, scenario 2) is an important use case because scenario 1) of running unit tests in Test Explorer will only run all the unit tests in the currently opened solution. We have multiple Visual Studio solutions each with their own sets of unit tests. So Scenario 2) is a way in which developers can quickly execute all unit tests (outside Visual Studio) across all solutions to make sure their changes did not break any unit tests.

Let me know what you think.
Thanks.

[csoltenborn]

Yes, your logic is correct, and that's the reason why I suggested to let the main method return 0.

I see. I have just tested the executable's behavior on the console, and it seems that memory leaks only cause an exit code of 1 if compiled in Debug mode (in Release mode they do not seem to have any effect), making matters even worse from GTA's point of view :-)

However, here's what you can could do:

• Configure GTA such that an additional parameter running_inside_gta is passed to the test executables
• Use that parameter inside your executable to decide whether to return the result of RUN_ALL_TESTS (in case the parameter is not passed) vs returning the result of the memory leak check (if any; if no leak check is performed, the test would still be green and thus not break the build).

[alfredskpoon]

Yes, CRT memory leak detection is only compiled for debug, and that is perfectly OK as it is expensive to perform and you wouldn't want it enabled in your optimized and released code. We use it in debug code as part of the Gated Build so memory leaks are never allowed to be checked into the code base.

[alfredskpoon]

So I really don't think we need to concern ourselves with memory leaks detection in release build if that will make matters slightly better for GTA :)

I think there are just 3 scenarios to consider, running with or without GTA:

1. All Tests Passed
2. At least 1 Test Failed
3. Memory Leak Detection Failure

For each scenario when running with GTA where a MemoryLeakTest is added,

1. All Tests Passed->test main returns 0->MemoryLeakTest pass
2. At least 1 Test Failed->test main returns 1->MemoryLeakTest fail
3. Memory Leak Detection Failure->test main returns 1->MemoryLeakTest fail

For each scenario when running without GTA (console) and no MemoryLeakTest is added,

1. All Tests Passed->test main returns 0
2. At least 1 Test Failed->test main returns 1
3. Memory Leak Detection Failure->test main returns 1

This is also why I still think test main should just return RUN_ALL_TESTS() (instead of always returning 0) to support case 2) when running without GTA.

I think in all these 6 cases, the outcome are acceptable, in the sense that if something has failed then overall status is failure and the build won't pass.

The only case that is slightly unexpected is case 2) with GTA, where a test failure also causes the MemoryLeakTest to show up as a failure. It's not too bad and I would be willing to live with that limitation. I ponder this some more though....

[csoltenborn]

I was surprised about the Release behavior because I usually try to minimize differences between Release and Debug behavior for the sake of better maintainability (and after all, I guess you are not delivering your test executables to your customers ;-) )... But that's of course completely up to you.

With the approach described above, we don't have to take that compromise. The main method will look something like this:

int result = RUN_ALL_TESTS();
if (running_inside_gta parameter is passed to test executable)
    return 0;
else
    return result;

That way, the executable will behave correctly if both run inside GTA and from the console. You could even use that switch to produce some meaningful output (e.g., only print the result code output tag if the switch is enabled, and print something like "No memory leak checks are performed in Release mode" in Release mode and the output of the memory leak check in Debug mode (if it as failed), so you can distinguish between the test having passed because no memory leaks have been found vs the test having passed because no check has been performed.

I will probably tonight provide a first version for testing - you can see at the sample code what I mean if you are unsure.

[alfredskpoon]

Thanks, I'm looking forward to testing it.

On a separate note, I uninstalled TAFGT yesterday and installed GTA in Visual Studio but ran into a very strange test discovery and execution issue. I don't think it is a problem in GTA as it would have been seen by many others using GTA. It may be related to my VS configuration but I can't see anything that is obviously wrong, and I'm wondering if you have come across this before. Sorry for being off topic and please let me know if I should move this to another forum.

Some of the discovered tests seem to "loose" its "test_case_name", and is left with just ".test_name" (instead of the full "test_case_name.test_name). It's hard to explain so I'll try and attach a few screenshots. I did not run into the same issues with TAFGA, and I was careful with exiting and restarting Visual Studio during the adapter re-installation. I can still run the tests in Test Explorer but have a lot fewer tests than there actually are.

[alfredskpoon]

I think I have found the issue after searching and reviewing issue 196,
#196

I have some initialization/shutdown code in test main that are run during test discovery. I didn't think they would interfere with test discovery, and they were not a problem with TAFGT but is certainly interfering with GTA. I've excluded the initialization/shutdown code from test discovery pass and all the tests are running now.

[csoltenborn]

I have provided a first version for testing. See C++ code and tests for usage.

There are some remaining issues (some of which I hope to be able to resolve with your help):

• The extra test is not yet treated correctly with respect to executing a selection of tests
• I have a test which causes a memory leak although it only contains an ASSERT_TRUE(false) statement (ASSERT_TRUE(true) seems to be working fine). I assume that this is a Google Test issue - have you experienced something similar?
• I wanted to produce some output in case no memory leak exists, but could not find an appropriate place for that.
  • Out of interest: the RunHook_() seems to only be called if there indeed are leaks. Why this check then?
  • Where can I place the according output?

[alfredskpoon]

Thanks for providing a beta for testing so quickly. I tested it this morning and it looks very good :)
I'll answer your questions first before giving feedback on testing.

• I see what you mean by the extra test not yet being treated correctly, see feedback below.

• Yes I have indeed experienced this memory leak issue in the google test framework when a test case fails due to an ASSERT_ failure, which was caught by the CRT memory leak checker. This is caused by false positives due to the google test framework not releasing statically held allocated memory on shutdown (Their threading makes it hard to correctly clean up all statically held allocated resources) This issue has been reported and addressed by the google test framework developers in the last year (see Fix/silence false positive memory leaks reported by Microsoft's debug CRT google/googletest#1142) by providing a RAII scoped object that excludes allocated memory (that are not real leaks) from being tracked by CRT memory leak checker. Unfortunately they have only fixed the memory leaks that occur during normal success operations, and there are some leaks (technically they are false positives) that occur during ASSERT failures which have not been fixed. If only they have added a memory leak test case for the failure scenario ;) I do have a fix for this memory leak in the google test framework but just haven't had time to submit a pull request to them yet :(

• I'm not sure what kind of output you can produce in case no memory leak exists, besides something like "Test Case passed". Bear in mind that the extra test case added by "Exit code test case" option may not even be a memory leak test case, as the flexibility is there to use the additional test case to catch any condition that triggers an abnormal exit code.

  • RunHook_() is just a function registered with the CRT debugging process, so it can be called for conditions other than memory leaks, thus the memory leak message comparison check to make sure when it is being called for a memory leak. When there is no memory leak, this hook function will not be called, thus my comment above that perhaps you don't need to provide additional output.

Now for feedback:

• I see that when I add an additional "MemoryLeakTest", my total number of tests grows by the number of tests processes I have, which was expected
• Expanding the test hierarchy however doesn't show the extra "MemoryLeakTest", unless there is a memory leak in which case it shows up as a failed test case. The total tests count (635) does not match up if all the individual tests are counted, but perhaps you are already aware of it.
• The simulated memory leaks are caught now, which is great :)
• There isn't much for the memory leak test failure message, besides "Message Exit Code 1", which is fine because the next step is to select the individual test case and debug it directly, which will break in the debugger and show more output. However, there is an issue (which you probably are already aware of)

I see the additional tests show up as "gtest_Solvers_exe.MemoryLeakTest" in the output, but in Test Explorer the hierarchy it looks like "gtest_Solvers.gtest_Solvers_exe.MemoryLeakTest".

Some more warnings I saw in output window,

[alfredskpoon]

Actually I was wrong about the extra "MemoryLeakTest" test not showing up in the test hierarchy if the test passed. Somehow I missed it in the screenshot above. I see that all test exe have an extra MemoryLeakTest.

[csoltenborn]

Thanks for the kudos! Btw, in case we get this running smoothly for your use cases: Your company might consider donating ;-)

I have just pushed a couple of bugfixes, should work better now. I may have let you test a bit too early, sorry for that - I must admit that I didn't have time to test within VS...

Gtest leaks: interesting... I encourage you to provide that pull request :-)

Output: I do not want to produce that output within GTA, but within the test executable. Currently, I provide output in case a leak occurs and if no leak detection is performed. It would feel more symmetric to provide some output such as "No memory leaks found." in that case.

As I see your comment on the message of the failing memoryleak test, I realize that you apparently haven't noted the two tags in the output which make GTA parse what follows and uses it as the test's message :-) I'd like to use that mechanism for the success case, too...

Btw, is there any way to get more output from leak detection? If that's possible, I'd like to add this to further improve the sample code.

[alfredskpoon]

For sure, you have been very proactive in providing support and I'll certainly take that into consideration!

Ahh, I did not notice the two tags in the output, I will incorporate that in our code and do some testing.

There are ways to get more detailed output from CRT leak detection. I did not supply that code initially as it would have taken longer to put together the memory leak test, but I'll put together something for you next week for your sample code.

[csoltenborn]

I've fixed the remaining issues as far as I can see - please try this version (note that the output tags have changed slightly). I've also put together some documentation.

As a side note: Given what I have so far, it shouldn't be too difficult to automatically identify the actual test(s) which produce leaks. Would that be an interesting use case for you?

[alfredskpoon]

Thanks, I'll try out the new version as soon as I can, in both Visual Studio and in automated Gated Builds, so it may be a few days before I have all the testing done, but I'll provide feedback as soon as it is available.

I'll also try and extract some of the memory leak message formatting code for you to use.

It would be nice to automatically identify the actual test(s) which produce the leaks (I assume you mean narrow it down to a single test or small number of tests within a test executable containing a large number of unit tests), but I don't see how you are able to do this.

The CRT memory leaks check runs at application shutdown after a set of test cases have run, and any one of those test cases could have caused the leak. The memory leak message formatting code which I'll provide you with can identify the source code location where the leaked memory was allocated, but it's not able to identify which unit test is associated with the memory leak. The only way I know of is to manually debug the tests in Visual Studio and use breakpoints to track down which test has a memory leak.

Anyway, if you know of a way to track down the leak to a finer granularity than the test executable let me know :)

[csoltenborn]

A recursive approach (split tests in half, run both sets of tests; mark all tests in runs without leaks as "not leaking"; mark all runs with a single test as leaking (if it does); repeat with all tests in runs with leaks; stop if all tests are marked "leaking" or "not leaking"). There are probably smarter ways to do this, but even this naive approach would probably be sufficiently fast (assuming that there are only few leaking tests). This would of course not happen during a test run, but could be invoked separately (e.g. for lunch break :-) ).

Anyways, if you start testing, please go for the PR build of this PR (see checks) to get the latest version...

[alfredskpoon]

Some feedback on 0.14.4.1268

• The warnings I noted previously are no longer seen
• However, I noticed that reporting of memory leaks is not consistent depending on how the test is run

In the screenshot below, I have a test executable, gtest_Solvers. The unit test fe_stress.tresca has a deliberate memory leak.

If I right click on "tresca" and pick "Run Select Tests" to run just this one test, then I see an expected MemoryLeakTest failure.

If I right click on "fe_stress" and pick "Run Select Tests" to run all tests in "fe_stress", I would expect "memoryLeakTest" to fail, but it passes in this case.

The same is seen if I run all tests in "gtest_Solvers", the "MemoryLeafTest" passes.

[csoltenborn]

That's surprising - a scenario like this behaves as desired with the SampleTests solution. I am tempted to think that your executable does not always return an appropriate exit code, I must admit...- I have just checked in some debug statements which log the exit code whenever a process execution is completed. Would you mind to double-check whether your executable returns the correct value in all cases?

Note that I have changed the naming scheme of the exit code tests - now the configured name comes first and is followed by the executable. This should imho result in nicer grouping and make the tests easier to find - let me know what you think...

[csoltenborn]

I forgot to mention: Switch on Debug mode to see the log output, and look for "returned with exit code".

[alfredskpoon]

Thanks, I did some more testing and there's something going on... The RunHook_ function that is called on memory leak detection (which calls exit(1)) is not getting called when I run the tests from the parent.

[csoltenborn]

Hm, that's not my fault, I guess... If this helps, have a look at the MemoryLeakTests project of the SampleTests solution - it works exactly as desired and is based on your code...

As a side note: Your probably are aware of the Print test output option!? This might help debugging your case. Let me know whether I can be of any further assistance (for instance, if you would like to have some more debug output)!

[alfredskpoon]

Yes, the memory leak detection should be triggered in all cases, I'll have to confirm that part first.
I'm enabling more debug output and doing some more testing, will let you know, thanks.

[alfredskpoon]

So after more testing using simulated memory leaks in different branches of the unit test hierarchy, I'm now certain the issue is related to side effects of some tests on one another causing the CRT memory leak checker not to be called. That's problems with our tests though. All that to say, I think the reporting of memory leaks through GTA to Test Explorer is good :)

I also tried 0.14.4.1270, and the rearrangement of the "MemoryLeakTest" to be higher in the test hierarchy is good. It makes it more prominent and noticeable.

I'll do some testing on the Build server in the next day or two.
Thanks.

[csoltenborn]

Ok, good luck with debugging your tests. FYI: I have just pushed another version where the exit code tests receive the executable's main method as source location.

[alfredskpoon]

I've added some more code to dump memory leak messages from CRT, just look at the differences in the RunHook_ function.
test_leak_v2.zip

[csoltenborn]

Thanks! I've just added it to my sample leak detection tests - weird stuff :-), but seems to be working nicely.

Any news from your side, e.g. concerning the build server? From my point of view, the feature is now pretty much ready for release...

[alfredskpoon]

I've been busy with other work and have not finished testing GTA on the build server yet. However, all the new functionality provided by GTA to support CRT memory leak detection have been tested from Visual Studio Test Explorer so I don't expect any issues. I should be done build server testing soon, so don't let me hold things up and feel free to release any time. Thanks again.

[csoltenborn]

Ok, take your time... I have pushed a couple of other small changes: Exit code tests can now also be skipped if they fail, and the tests can also be run without "real" tests. Finally, I have refactored the memory leak detection code such that it can easily be reused.

Which leads to the following question: Is there anything I should let my users know in case they indeed want to reuse leak detection for their own test projects? For instance, it seems that only memory allocated using new will be tracked (no malloc() calls) - is this true?

Could you maybe (if your time allows) quickly browse through the code (leak detection, invocation) I came up with, and maybe even the docs?

[alfredskpoon]

Sorry for the delay, I've managed to verify memory leak detection using the new GTA ExitCodeTest functionality on a Gated Build running on a build server. Everything is working.

I have also browsed through the leak detection and invocation code you referenced above, the only comment I have is where you write out the GTA marker tag,

std::cout << "GTA_EXIT_CODE_OUTPUT_BEGIN\n";

During testing I found that this tag was not detected by GTA in time for it to extract the CRT memory leak messages. The differences in behaviour may be related to timing differences between your test application and our code. I found that I had to flush the output to ensure the logs are captured, by adding,

std::flush(std::wcout);

So you may want to include this in your sample code to ensure consistent behaviour. This was a bit tricky to resolve.

The link to your docs above is not working, although I did look through the docs initially when it was working and did not see anything incorrect. If you have updated the docs since then I can take another look.

BTW, if you don't mind me asking, how far off are you from reaching your goal of funding your replacement laptop? and feel free to send me a private message.

[csoltenborn]

Thanks for your feedback, and for tracking down that "output issue"! Where did you have to put the std::flush() statement?

Afaik, I can't send private messages through GitHub... Would you mind dropping me an email at my first name at my last name dot de?

[alfredskpoon]

The std::flush statement should immediately follow the output of GTA_EXIT_CODE_OUTPUT_BEGIN tag.

Thanks, I'll send you a direct email.