Solved

problem with AtlWinModuleTerm and free() called in pWinModule->m_rgWindowClassAtoms.RemoveAll();

Posted on 2006-07-18
12
602 Views
Last Modified: 2008-02-20

Ok.  Something I changed in my code recently is giving me a heap error when my ATL module tries to terminate.

ATL::AtlWinModuleTerm calls pWinModule->m_rgWindowClassAtoms.RemoveAll();

m_rgWindowClassAtoms is a CSimpleArray and remove all looks like this:

      void RemoveAll()
      {
            if(m_aT != NULL)
            {
                  for(int i = 0; i < m_nSize; i++)
                        m_aT[i].~T();
                  free(m_aT);   // The Offending "free"
                  m_aT = NULL;
            }
            m_nSize = 0;
            m_nAllocSize = 0;
      }

Unfortunately, when it tries to free(m_aT), it asserts.  The assert is from the following code:

        /*
         * If this ASSERT fails, a bad pointer has been passed in. It may be
         * totally bogus, or it may have been allocated from another heap.
         * The pointer MUST come from the 'local' heap.
         */
        _ASSERTE(_CrtIsValidHeapPointer(pUserData));

However, the offending window class (I've narrowed down the possibilities so I know exactly which class is causing the problem) is defined with CWindowImpl and uses the DECLARE_WND_CLASS macro.  

     CWindowImpl<CMyWindow,CWindow,CWinTraits<WS_DLGFRAME | WS_POPUP | WS_CLIPCHILDREN | WS_CLIPSIBLINGS, 0> >

DECLARE_WND_CLASS(_T("JoesWindowClass"));

In short, I'm not managing rgWindowClassAtoms, ATL should be.  

If I comment out the "free(m_aT)", the code works just fine.  But I'm sure that's a memory leak for the rest of the CSimpleArrays out there. (Not sure if it matters for the Term(), but it probably does).

I'm using ATL 7.0 and the only clear insight I've found so far online is a write up that suggests both the underlying problem and a work-around.

http://discuss.develop.com/archives/wa.exe?A2=ind0202d&L=dotnet&D=1&T=0&F=&S=&P=72637

However, I /am/ statically linking to ATL as near as I can tell, so I should already have the workaround in place.

I'm really not sure what to do. For the moment I'm planning on re-writing the ATL files to avoid the bug, and accept the memory leak until I can figure out a way around this problem.

Anyone have a solution that isn't a memory leak?

-Joe
0
Comment
Question by:jandrieu
  • 7
  • 4
12 Comments
 
LVL 15

Expert Comment

by:lakshman_ce
ID: 17130964
Check to see if your code is calling Init/Term methods.

In ATL 7.0.,Init and Term methods have moved into the constructors and destructors for the module classes; there is no longer a need to call Init and Term.

0
 

Author Comment

by:jandrieu
ID: 17132014
Good idea.  No luck.  

The only hits for a search on Term( and Init( are in the ATL files plus a CComObject I initialize in a customized IConnectionPointImplMT<T, piid, CDV>::EnumConnections.
0
 
LVL 15

Expert Comment

by:lakshman_ce
ID: 17132765
I am not sure if
DECLARE_WND_CLASS(_T("JoesWindowClass"));
is causing you the problem.

You can use the DECLARE_WND_CLASS_EX macro to provide your own styles and background color.



0
 

Author Comment

by:jandrieu
ID: 17132991
That didn't seem to help either.

I tried:
DECLARE_WND_CLASS_EX("JoesWindowClass",NULL,NULL)

Which compiled & ran, but still left the heap bug.

Also:
DECLARE_WND_CLASS_EX("JoesWindowClass",WS_DLGFRAME | WS_POPUP | WS_CLIPCHILDREN | WS_CLIPSIBLINGS,NULL)

Which game a parameter error (as expected, actually).

And then I commented out the line entirely.

That didn't work either.
0
 
LVL 15

Expert Comment

by:lakshman_ce
ID: 17133222
Interesting...

Btw what changes you made? Was it all working fine without your changes?
0
 

Author Comment

by:jandrieu
ID: 17134482
Unfortunately, it's not clear what changes made the difference or when.  I had the program working fine before I started implementing Drag & Drop into/from my own MSHTML CAxWindow, adding several new COM classes and figuring out how to get that to work.  That was causing a lot of crashes on its own, so I didn't pay attention at first when the app started crashing at shutdown.

Once I got the drag & drop working, I started paying attention to the heap bug.  In doing so, I isolated out what I had added bit by bit. I now have the heap problem, but NONE of my new classes are being used.  I don't even include their .h files.  I had thought it might be something with my IDataObject implementation, misusing STGMEDIUM, but it happens without doing ANY drag & drop. I don't even register the drag & drop at this point.

One set of changes was moving from hand-written registry functions to the ATL macros.  I've been working with a variety of sources for sample code and my earliest code handled DllRegisterServer manually.  Now I use the ATL DllRegisterServer().

I look into that and see if I can change anything.

-j
0
Why You Should Analyze Threat Actor TTPs

After years of analyzing threat actor behavior, it’s become clear that at any given time there are specific tactics, techniques, and procedures (TTPs) that are particularly prevalent. By analyzing and understanding these TTPs, you can dramatically enhance your security program.

 

Author Comment

by:jandrieu
ID: 17134621
Hmph.

So I went back to my old RegisterServer functions and even excised my new classes out of the IDL and the VS project.

No luck.
0
 
LVL 15

Accepted Solution

by:
lakshman_ce earned 500 total points
ID: 17138148
I would suggest you to take the base code that you had initially and see if heap problem exists in that code.
If it exists you can check this link to see if you are missing anything.
http://www.codeproject.com/wtl/wtl4mfc1.asp?df=100&forumid=14959&exp=0&select=719285

0
 
LVL 49

Expert Comment

by:DanRollins
ID: 17142317
I'm pretty sure that the main reason to get that assert is that the item being freed has already been freed.

I'd breakpoint the code in CSimpleArray to see if something else has already deleted it.

I'm sorry if this offends you but there is also another possibility:  
You may be thinking the wrong line is causing the problem.  Whenever I write a for... loop, I use curly braces and one happy side effect is that it is always very clear which line is being executed when single-stepping:

               for(int i = 0; i < m_nSize; i++) {       // added
                    m_aT[i].~T();
                }                                                  // added
               free(m_aT);   // The Offending "free"

Note too, that you are using m_nSize to determine how many items to free.  I'd recommend using the GetSize() member function.

-- Dan
0
 

Author Comment

by:jandrieu
ID: 17142539
As far as I can tell, the array is valid.  (I've gone back in history with SourceSafe so I can't test that directly anymore.)  However, the m_AT[i].~T(); always worked fine, which means that m_aT must be valid.

I did, when I was debugging it before, walk through and document all the pointers that were freed with this particular function.  None of the pointer values were repeated.  My guess is that m_aT is on a different heap.

As for the braces.  I agree. This, however, is code straight from the ATL library.  I'll avoid updating it if possible.

-joe
0
 

Author Comment

by:jandrieu
ID: 17143986
Here's the latest, in case anyone is still following this thread:

Apparently, I'm thrashing the heap somewhere. That's my best guess for the assert.
I also know that the OnFinalMessage for CMyWindow is getting called, but the destructor is not.

I had return in time to a place where I wasn't causing problems, then slowly went forward, adding code. Only to find that when the bug resurfaced, the latest changes were apparently not the cause.  Grrr. I've turned off pre-compiled headers in case there was something with that. Suffice to say I'm back in the midst of the heap bug.

-j
0
 

Author Comment

by:jandrieu
ID: 17223066
Ultimately, I rebuilt the underlying codebase from the ground up (for this component). That cleaned up some potential issues, but I still have no idea what was causing the crash.

I've awarded points to the best advice, but since it wasn't a specific answer to the problem but rather a general strategy for working around the problem, I gave a grade of C.

But, at the end of the day, it was still the right answer. Chasing after phantom bugs sucks.  At least now the codebase is a little cleaner and hopefully a bit more stable.

-j
0

Featured Post

Free Trending Threat Insights Every Day

Enhance your security with threat intelligence from the web. Get trending threat insights on hackers, exploits, and suspicious IP addresses delivered to your inbox with our free Cyber Daily.

Join & Write a Comment

Suggested Solutions

The following diagram presents a diamond class hierarchy: As depicted, diamond inheritance denotes when two classes (e.g., CDerived1 and CDerived2), separately extending a common base class (e.g., CBase), are sub classed simultaneously by a fourt…
In Easy String Encryption Using CryptoAPI in C++ (http://www.experts-exchange.com/viewArticle.jsp?aid=1193) I described how to encrypt text and recommended that the encrypted text be stored as a series of hexadecimal digits -- because cyphertext may…
This video discusses moving either the default database or any database to a new volume.
In this seventh video of the Xpdf series, we discuss and demonstrate the PDFfonts utility, which lists all the fonts used in a PDF file. It does this via a command line interface, making it suitable for use in programs, scripts, batch files — any pl…

705 members asked questions and received personalized solutions in the past 7 days.

Join the community of 500,000 technology professionals and ask your questions.

Join & Ask a Question

Need Help in Real-Time?

Connect with top rated Experts

19 Experts available now in Live!

Get 1:1 Help Now