asked on

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

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

lakshman_ce

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.

jandrieu

ASKER

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.

lakshman_ce

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.

jandrieu

ASKER

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.

lakshman_ce

Interesting...

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

jandrieu

ASKER

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

jandrieu

ASKER

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.

ASKER CERTIFIED SOLUTION

lakshman_ce

membership

This solution is only available to members.

To access this solution, you must be a member of Experts Exchange.

Start Free Trial

DanRollins

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

jandrieu

ASKER

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

jandrieu

ASKER

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

jandrieu

ASKER

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

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

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