Application breaks on CCriticalSection::Lock

Question

I am upgrading an application from VC6 to VS2010 (Legacy Code). The application runs as it should in VC6 but after converting the project to VS2010 I encountered some problems. On exiting the application, the program breaks while attempting to lock on entering a critical section. The lock count usually alternates from -1(Unlocked) to -2(Locked) but just before the program crashes, the lock count is 0.

g_RenderTargetCriticalSection.Lock();// Breaks here

if (g_RenderTargets.Lookup(this, pRenderTarget))
{
    ASSERT_VALID(pRenderTarget);
    g_RenderTargets.RemoveKey(this);
    delete pRenderTarget;
}

g_RenderTargetCriticalSection.Unlock();

Here is the CCriticalSection::Lock() function where ::EnterCriticalSection(&m_sect); fails. I found it strange that on failing, the lock count changes from 0 to -4??

_AFXMT_INLINE BOOL (::CCriticalSection::Lock())
{   
    ::EnterCriticalSection(&m_sect); 

    return TRUE; 
}

If anyone has encountered anything similar to this, some insight would be greatly appreciated. Thanks in advance.

Answer 1

The issue was that the application's Main Window was being destroyed after the application's global object was destroyed. This meant that the g_renderTargetCriticalSection was already Null when the main window was trying to be destroyed.

The solution was to destroy the Application's main window before it's global object (CDBApp theApp) calls ExitInstance() and is destroyed.

int CDBApp::ExitInstance() 
{
    LOGO_RELEASE
    //Destroy the Main Window before the CDBApp Object (theApp) is destroyed.
    if(m_Instance.m_hWnd)
        m_Instance.DestroyWindow();

    return CWinApp::ExitInstance();
}

Answer 2

The comments indicate this is a file-scope object destructor order issue. There are various ways you could address this. Since I haven't seen the rest of the code it's difficult to offer specific advice, but one idea is to change the CS to live in a shared_ptr and have your CWnd hold onto a copy so it won't be destroyed prematurely. e.g.:

std::shared_ptr<CCriticalSection> g_renderTargetCriticalSection(new CCriticalSection());

Then in your window class:

class CMyWindow : public CWnd
{
private:
   std::shared_ptr<CCriticalSection> m_renderTargetCriticalSection;

public:
   CMyWindow()
      : m_renderTargetCriticalSection(g_renderTargetCriticalSection)
   {
       // ...
   }

   ~CMyWindow()
   {
      // guaranteed to still be valid since our shared_ptr is keeping it alive
      CSingleLock lock(m_renderTargetCriticalSection.get(), TRUE);
      // ...
   }

   // ...
};

Answer 3

This code doesn't make sense:

int CDBApp::ExitInstance() 
{
    LOGO_RELEASE
    //Destroy the Main Window before the CDBApp Object (theApp) is destroyed.
    if(m_Instance.m_hWnd)
        m_Instance.DestroyWindow();

    return CWinApp::ExitInstance();
}

m_Instance is a handle, not a class, so it can't be used to call functions!