0

I am creating a modeless dialog box. The dialog box is called from the menu item of main frame window.

MainFrm.h

CModeless* modeless;    
bool modelessDlgOpen;     

MainFrm.cpp

void CMainFrame::OnDatabaseMLdlg()     
{     
    // TODO: Add your command handler code here     
    if (modelessDlgOpen == TRUE)      
        return;     

    modelessDlgOpen = TRUE;     

    modeless = new CModeless(this);     

    //modeless->Create(IDD_MLDLG, GetDesktopWindow());     
    modeless->Create(IDD_MLDLG, this);     
    mbPoll->ShowWindow(SW_SHOW);     
}     

When menu item is clicked, OnDatabaseMLdlg() function is called and a modeless dialog box with resource ID IDD_MLDLG appears.

The issue is while closing the modeless dialog box.

I am not able to find out the correct method to have a clean closure / destroy of this modeless dialog box. Upon clicking the cross button in right-top corner, which message gets generated?

My current code which I have tried is as follows. (producing code related only to the closure of the dialog box)

MLDLG.h

#pragma once     

#define WM_MLDLG_CLOSED (WM_USER + 555)     
// CModeless dialog     

class CModeless : public CDialog     
{     
    DECLARE_DYNAMIC(CModeless)     

public:     
    CModeless(CWnd* pParent = NULL);   // standard constructor     
    virtual ~CModeless();      

// Dialog Data     
    enum { IDD = IDD_MLDLG };      

protected:     
    virtual void DoDataExchange(CDataExchange* pDX);    // DDX/DDV support      

    DECLARE_MESSAGE_MAP()     
public:     
    virtual BOOL Create(UINT nIDTemplate, CWnd* pParentWnd = NULL);     
    afx_msg void OnNcDestroy();     
    virtual void PostNcDestroy();     
    CWnd* mParent;     
    afx_msg void OnBnClickedCancel();     
};     

MLDLG.cpp

void CModeless::OnNcDestroy()     
{     
    CDialog::OnNcDestroy();     

    // TODO: Add your message handler code here     
}     

void CModeless::PostNcDestroy()      
{        
    CDialog::PostNcDestroy();     
    GetParent()->PostMessage(WM_MLDLG_CLOSED,0,0);     // **CRASHES HERE**
    delete this;     
}     

void CModeless::OnBnClickedCancel()     
{     
    // TODO: Add your control notification handler code here     
    //CDialog::OnCancel();     
    DestroyWindow();         
}     

Not able to understand what am I doing wrong or what am I missing?

I can provide additional details in case required.

Thanks in advance.

EDIT-20130612: Additional information:

My constructor is as follows:

CModeless::CModeless(CWnd* pParent /*=NULL*/)    
    : CDialog(CModeless::IDD, pParent)    
{    
    mParent = pParent;    
    if (mParent == NULL)    
    {    
        MessageBox(L"mParent is NULL");   
    }    
    else    
    {    
        MessageBox(L"mParent is not NULL");    
    }    
}    

Here, I have verified that mParent is not NULL.

Jay
  • 1,210
  • 9
  • 28
  • 48
  • 1
    Does GetParent() return NULL? If so, your override of the dialog Create function is failing to pass the pParent to the CDialog constructor. If you are going to post a message to the parent then you must save the pParent, and make sure it is not NULL, one way or another. – ScottMcP-MVP Jun 11 '13 at 19:28
  • In the constructor I am saving pParent variable to a member variable CWnd* mParent. I thought GetParent would return the same and is safer than something like mParent-> PostMessage(). Please correct me if i'm wrong. – Jay Jun 11 '13 at 19:39
  • 1
    GetParent gets a property of the window. It certainly does not get your mParent member variable. That property is set by the CDialog::Create pParentWnd parameter. Whether you use your own member variable or the dialog's pParentWnd (either way will work) you have to make sure it is not NULL. – ScottMcP-MVP Jun 12 '13 at 02:24
  • @ScottMcP-MVP: I replaced my GetParent()->PostMessage(WM_MLDLG_CLOSED,0,0); call with mParent->PostMessage(WM_MLDLG_CLOSED,0,0); And it seems to work fine. Is it the correct way to do it?? GetParent() returns NULL. – Jay Jun 12 '13 at 03:45
  • 1
    CModeless(CWnd* pParent = NULL); Using mParent->PostMessage(...); is OK. But you are leaving behind a time bomb by giving the constructor a default NULL argument. You should remove the = NULL so it won't work if somebody tries to use the default version in the future. – ScottMcP-MVP Jun 13 '13 at 01:45
  • @ScottMcP-MVP: Yes you are right. Just have a look at the question which I edited yesterday to add my constructor code. Default NULL is already commented. – Jay Jun 13 '13 at 03:15
  • You may also need ASSERT condition in the constructor to ensure parent is not NULL – Naga Jul 14 '15 at 16:19

1 Answers1

0

PostNCDestroy() is called VERY late and most of the useful state of the MFC window is not valid at that point. GetParent() is probably returning NULL, which will cause a crash the way you are using it.

Try moving the PostMessage call to OnDestroy() before calling the base class implementation there.

Another option is to cache the parent's hWnd and call ::PostMessage() on that hWnd;

edtheprogrammerguy
  • 5,957
  • 6
  • 28
  • 47
  • 2
    Your advice here is wrong and dangerous. PostNcDestroy is the last message received by the dialog so it is the only safe member function to do 'delete this.' Deleting any sooner causes the deleted object to attempt to handle a message. The CDialog documentation says "You should override PostNCDestroy for modeless dialog boxes in order to delete this." – ScottMcP-MVP Jun 12 '13 at 02:31
  • I didn't mean to move the call to `delete this`, only to move his `PostMessage` call to `OnDestroy`. Sorry for the confusion. – edtheprogrammerguy Jun 12 '13 at 12:29