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

Macro ACE_NO_HEAP_CHECK (ACE_No_Heap_Check ____no_heap) disables memory tracking for all threads #733

Closed
vbaderks opened this issue Oct 30, 2018 · 1 comment

Comments

@vbaderks
Copy link

Version

ACE 6.5.1, problem also applies to 6.5.2

Host machine and operating system

Windows 10 64 bit

Note: problem is limited to applications that use the Microsoft C\C++ runtime library on the Windows platform.

Compiler name and version (including patch level)

Microsoft (R) C/C++ Optimizing Compiler Version 19.15.26732.1 for x86

AREA/CLASS/EXAMPLE AFFECTED:

global_macros.h

The problem effects:

The debug build of the ACE library disables the memory tracking feature of the Microsoft C\C++ runtime library.

Synopsis

The ACE library uses the ACE_NO_HEAP_CHECK macro at a couple of places to disable/enable memory tracking for a local function, however this design fails for multi-threading applications.

Description

The ACE library provides a class and macro to disable temporarily the memory leak tracking feature of the debug build of the Microsoft C\C++ runtime:

#define ACE_NO_HEAP_CHECK ACE_No_Heap_Check ____no_heap;

class ACE_Export ACE_No_Heap_Check
{
public:
ACE_No_Heap_Check (void)
: old_state (_CrtSetDbgFlag (_CRTDBG_REPORT_FLAG))
{ _CrtSetDbgFlag (old_state & ~_CRTDBG_ALLOC_MEM_DF);}
~ACE_No_Heap_Check (void) { _CrtSetDbgFlag (old_state);}
private:
int old_state;
};

The intend of the macro is that it should be used when a function is entered, to suppress false leaks. At function exit the original tracking state is restored.

However the concept of this design is flawed:

  1. As long as the ACE_Export ACE_No_Heap_Check instance exists in one thread, all other memory allocations in other threads are also excluded from the memory tracking. All leakage during that time period are not reported by the CRT debug report when the application is exited.

  2. If another thread uses also the ACE_NO_HEAP_CHECK ACE_No_Heap_Check macro when the tracking is already disabled by another ACE_NO_HEAP_CHECK ACE_No_Heap_Check macro, the old_state variable that will be retrieved to restore is the "disabled" state. Depending on the timing which destructor is executed first, the memory tracking is disabled completely for the remaining lifetime of the app. For example:
    Thread 1: ACE_NO_HEAP_CHECK ACE_No_Heap_Check constructor -> disables memory tracking
    Thread 2: ACE_NO_HEAP_CHECK ACE_No_Heap_Check constructor -> disables memory tracking, (old_state = disabled)
    Thread 1: ACE_NO_HEAP_CHECK ACE_No_Heap_Check destructor -> enables memory tracking
    Thread 2: ACE_NO_HEAP_CHECK ACE_No_Heap_Check destructor -> disables memory tracking
    Thread 3: ACE_NO_HEAP_CHECK ACE_No_Heap_Check constructor -> disables memory tracking
    Thread 3: ACE_NO_HEAP_CHECK ACE_No_Heap_Check destructor -> disables memory tracking

Repeat by

  1. Add a explicit memory leak somewhere in code that is executed by a worker thread.
  2. Execute the application (debug build)
  3. Close the application
    Expected result: the MS CRT gives a list of leaked memory blocks.
    Actual result: no leaks are reported.

Sample fix/ workaround

My recommendation is remove the ACE_NO_HEAP_CHECK ACE_No_Heap_Check macro and the class ACE_Export ACE_No_Heap_Check to solve this issue.
If there are memory leaks reported afterwards (created by global singletons, for example), it means that additional modifications are needed to remove those, currently hidden, leaks.

Note: the Microsoft C runtime also provides a special debug version of malloc_dbg (internally used by the memory tracking system) that accept a blockType parameter. Passing "IGNORE_BLOCK" means that the leaked memory will not be reported as leaked. This could be an alternative solution to suppress leak warnings, when it is difficult\impossible to modify the leaking implementation.

jwillemsen added a commit to jwillemsen/ATCD that referenced this issue Feb 6, 2019
…oup#733

    * ACE/ace/Global_Macros.h:
    * ACE/ace/Log_Msg.cpp:
    * ACE/ace/OS_NS_Thread.cpp:
    * ACE/ace/Service_Gestalt.cpp:
jwillemsen added a commit that referenced this issue Feb 7, 2019
Removed ACE_NO_HEAP_CHECK macro because it is broken, see issue #733
@jwillemsen
Copy link
Member

Thanks for reporting, fixed by PR #831

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

No branches or pull requests

2 participants