Memory leaks

[ikolev]

Hi guys,

First, thanks for this great control.
We're using it on Windows with Visual C++ 2008. We also use the option of VC's runtime library to report memory leaks at the application's exit, like this:

Code: Select all

_CrtSetDbgFlag( _CRTDBG_LEAK_CHECK_DF | _CrtSetDbgFlag(_CRTDBG_REPORT_FLAG) )

We found that using the wxWebControl adds a lot of leaks to the output. They are not important leaks - just some services are registered in the beginning and not unregistered at exit. But they mess with the leaks list and make it very hard to spot our real leaks.
We tried to trace the leaks, but we don't know anything about XPCOM and we are not sure what we are doing. For example, we found that adding the following code to ~GeckoEngine() avoids the leak of m_plugin_provider:

Code: Select all

ns_smartptr<nsIDirectoryService> dir_service = nsGetDirectoryService();
nsresult res = dir_service->UnregisterProvider(m_plugin_provider);

Another source of leaks seem to be the RegisterFactory() calls (there are no corresponding UnregisterFactory() calls).

Would it be possible to clear these in the original sources?

Thanks,
Ivan

[Ben]

These leaks are caused by the Gecko engine. I've tried to resolve them, but there is apparently nothing that can be done about it. As soon as the Gecko engine is loaded, you'll have those leak reports.

Best,
Ben

[Lance7_q]

Hi,
Take a look on these:

Microsoft's Internet Explorer contains a number of leaks, the worst of which is an interaction with JScript. When a DOM object contains a reference to a JavaScript object (such an event handling function), and when that JavaScript object contains a reference to that DOM object, then a cyclic structure is formed. This is not in itself a problem. At such time as there are no other references to the DOM object and the event handler, then the garbage collector (an automatic memory resource manager) will reclaim them both, allowing their space to be reallocated. The JavaScript garbage collector understands about cycles and is not confused by them. Unfortunately, IE's DOM is not managed by JScript. It has its own memory manager that does not understand about cycles and so gets very confused. As a result, when cycles occur, memory reclamation does not occur. The memory that is not reclaimed is said to have leaked. Over time, this can result in memory starvation. In a memory space full of used cells, the browser starves to death.

Regards!

[jonmmorgan]

The list of leaks given in the initial post do not appear to be DOM leaks (which can be very bad), but merely things registered at the start and not freed at the finish. As these things must be registered for the entire running of the application, and as Gecko / the OS will free them on application exit, they are not important leaks. Even if they were freed by wxWebConnect they would still be allocated for the entire running period of the application, and so freeing them at the end would have no positive effect on application performance for the lifetime of the application.

[HansDR]

@jonmorgan

While you are technically correct, these leaks make it hard to spot other leaks which could be more serious. Are there new memory leaks in the list? Or are they all caused by using webconnect? That's hard to tell when there's no way of knowing what module caused them. With no pre-existing leaks, any new leaks are immediately obvious.

Anyway, I have managed to eliminate all of the memory leaks. Ikolev was on the right track when he said that the leaks were being caused by objects not being unregistered. I have discovered that, on cleanup, XULRunner/XPCOM doesn't release the objects that you allocated and passed to it; hence, their destructors never get called (their usage counters never get to zero) and you end up with memory leaks. This means that you have to unregister everything that you register.

Here's my patch against the 1.1 release:

120a121,126
>
> ns_smartptr<nsIComponentRegistrar> comp_reg;
> ns_smartptr<nsIFactory> prompt_factory;
> ns_smartptr<nsIFactory> nssdialogs_factory;
> ns_smartptr<nsIFactory> transfer_factory;
> ns_smartptr<nsIFactory> unknowncontenttype_factory;
1559a1566,1593
> if(comp_reg)
> {
> nsCID prompt_cid = NS_PROMPTSERVICE_CID;
> comp_reg->UnregisterFactory(prompt_cid,
> prompt_factory);
>
> nsCID nssdialogs_cid = NS_NSSDIALOGS_CID;
> comp_reg->UnregisterFactory(nssdialogs_cid,
> nssdialogs_factory);
>
> nsCID download_cid = NS_DOWNLOAD_CID;
> comp_reg->UnregisterFactory(download_cid,
> transfer_factory);
>
> comp_reg->UnregisterFactory(download_cid,
> transfer_factory);
>
> nsCID unknowncontenthtypehandler_cid = NS_UNKNOWNCONTENTTYPEHANDLER_CID;
> comp_reg->UnregisterFactory(unknowncontenthtypehandler_cid,
> unknowncontenttype_factory);
> }
>
> ns_smartptr<nsIDirectoryService> dir_service = nsGetDirectoryService();
> if(dir_service)
> {
> nsresult res = dir_service->UnregisterProvider(m_plugin_provider);
> dir_service.clear();
> }
1560a1595,1602
>
> // Final task, get the window_watcher to release the WindowCreator object
> // so that its destructor gets called and it gets destructed cleanly
> ns_smartptr<nsIWindowWatcher> window_watcher = nsGetWindowWatcherService();
> if (window_watcher)
> {
> window_watcher->SetWindowCreator(NULL);
> }
1699d1740
< ns_smartptr<nsIComponentRegistrar> comp_reg;
1704c1745
< ns_smartptr<nsIFactory> prompt_factory;
---
>
1712,1714c1753,1754
<
< prompt_factory.clear();
< CreatePromptServiceFactory(&prompt_factory.p);
---
>
> CreatePromptServiceFactory(&nssdialogs_factory.p);
1720c1760
< prompt_factory);
---
> nssdialogs_factory);
1724d1763
< ns_smartptr<nsIFactory> transfer_factory;
1738,1739d1776
<
< ns_smartptr<nsIFactory> unknowncontenttype_factory;


The changes are as follows:

Add the following private fields to the GeckoEngine class:

Code: Select all

    ns_smartptr<nsIComponentRegistrar> comp_reg;
    ns_smartptr<nsIFactory> prompt_factory;
    ns_smartptr<nsIFactory> nssdialogs_factory;
    ns_smartptr<nsIFactory> transfer_factory;
    ns_smartptr<nsIFactory> unknowncontenttype_factory;


Change the GeckoEngine's destructor code to:

Code: Select all

   if(comp_reg)
   {
      nsCID prompt_cid = NS_PROMPTSERVICE_CID;
      comp_reg->UnregisterFactory(prompt_cid,
                              prompt_factory);
      
      nsCID nssdialogs_cid = NS_NSSDIALOGS_CID;
      comp_reg->UnregisterFactory(nssdialogs_cid,
                              nssdialogs_factory);

      nsCID download_cid = NS_DOWNLOAD_CID;
      comp_reg->UnregisterFactory(download_cid,
                              transfer_factory);
                                      
      comp_reg->UnregisterFactory(download_cid,
                              transfer_factory);

      nsCID unknowncontenthtypehandler_cid = NS_UNKNOWNCONTENTTYPEHANDLER_CID;
      comp_reg->UnregisterFactory(unknowncontenthtypehandler_cid,
                              unknowncontenttype_factory);
   }

   ns_smartptr<nsIDirectoryService> dir_service = nsGetDirectoryService();
   if(dir_service)
   {
      nsresult res = dir_service->UnregisterProvider(m_plugin_provider);
        dir_service.clear();
   }
    m_plugin_provider->Release();

    // Final task, get the window_watcher to release the WindowCreator object
    // so that its destructor gets called and it gets destructed cleanly
    ns_smartptr<nsIWindowWatcher> window_watcher = nsGetWindowWatcherService();
    if (window_watcher)
   {
        window_watcher->SetWindowCreator(NULL);
   }

Change GeckoEngine's Init() method to:

Code: Select all

    nsresult res;

    if (IsOk())
        return true;
   
    if (m_gecko_path.IsEmpty())
        return false;
   
    if (m_storage_path.IsEmpty())
    {
        wxLogNull log;

        wxString default_storage_path = wxStandardPaths::Get().GetTempDir();
        wxChar path_separator = wxFileName::GetPathSeparator();
        if (default_storage_path.IsEmpty() || default_storage_path.Last() != path_separator)
            default_storage_path += path_separator;
        default_storage_path += wxT("kwkh01.tmp");
       
#ifdef WIN32
        ::wxMkDir(default_storage_path);
#else
        ::wxMkDir((const char*)default_storage_path.mbc_str(), 0700);
#endif

        m_storage_path = default_storage_path;
    }

    SetStoragePath(m_storage_path);

   
    char path_separator = (char)wxFileName::GetPathSeparator();
    std::string gecko_path = (const char*)m_gecko_path.mbc_str();
    std::string xpcom_path = gecko_path;
    if (xpcom_path.empty() || xpcom_path[xpcom_path.length()-1] != path_separator)
        xpcom_path += path_separator;
    #if defined __WXMSW__
    xpcom_path += "xpcom.dll";
    #elif defined __WXMAC__
    xpcom_path += "libxpcom.dylib";
    #else
    xpcom_path += "libxpcom.so";
    #endif

       
    if (NS_FAILED(XPCOMGlueStartup(xpcom_path.c_str())))
        return false;
   
    ns_smartptr<nsILocalFile> gre_dir;
    res = NS_NewNativeLocalFile(nsDependentCString(gecko_path.c_str()), PR_TRUE, &gre_dir.p);
    if (NS_FAILED(res))
        return false;

    if (NS_FAILED(NS_InitXPCOM2(nsnull, gre_dir, nsnull)))
        return false;
   
   
    // create an app shell
    const nsCID appshell_cid = NS_APPSHELL_CID;
    m_appshell = nsCreateInstance(appshell_cid);
    if (m_appshell)
    {
        m_appshell->Create(0, nsnull);
        m_appshell->Spinup();
    }

    // set the window creator
   
    ns_smartptr<nsIWindowWatcher> window_watcher = nsGetWindowWatcherService();
    if (!window_watcher)
        return false;
       
    ns_smartptr<nsIWindowCreator> wnd_creator = static_cast<nsIWindowCreator*>(new WindowCreator);
    window_watcher->SetWindowCreator(wnd_creator);


    // set up our own custom prompting service
   
    res = NS_GetComponentRegistrar(&comp_reg.p);
    if (NS_FAILED(res))
        return false;
   
   
    CreatePromptServiceFactory(&prompt_factory.p);

    nsCID prompt_cid = NS_PROMPTSERVICE_CID;
    res = comp_reg->RegisterFactory(prompt_cid,
                                    "Prompt Service",
                                    "@mozilla.org/embedcomp/prompt-service;1",
                                    prompt_factory);
   
    CreatePromptServiceFactory(&nssdialogs_factory.p);
   
    nsCID nssdialogs_cid = NS_NSSDIALOGS_CID;
    res = comp_reg->RegisterFactory(nssdialogs_cid,
                                    "PSM Dialog Impl",
                                    "@mozilla.org/nsBadCertListener;1",
                                    nssdialogs_factory);

    // set up our own download progress service
   
    CreateTransferFactory(&transfer_factory.p);

    nsCID download_cid = NS_DOWNLOAD_CID;
    res = comp_reg->RegisterFactory(download_cid,
                                    "Transfer",
                                    "@mozilla.org/transfer;1",
                                    transfer_factory);
                                   
    res = comp_reg->RegisterFactory(download_cid,
                                    "Transfer",
                                    "@mozilla.org/download;1",
                                    transfer_factory);

    CreateUnknownContentTypeHandlerFactory(&unknowncontenttype_factory.p);

    nsCID unknowncontenthtypehandler_cid = NS_UNKNOWNCONTENTTYPEHANDLER_CID;
    res = comp_reg->RegisterFactory(unknowncontenthtypehandler_cid,
                                    "Helper App Launcher Dialog",
                                    "@mozilla.org/helperapplauncherdialog;1",
                                    unknowncontenttype_factory);
                                   
/*
    // set up cert override service
   
    ns_smartptr<nsIFactory> certoverride_factory;
    CreateCertOverrideFactory(&certoverride_factory.p);
   
    nsCID certoverride_cid = NS_CERTOVERRIDE_CID;
    res = comp_reg->RegisterFactory(certoverride_cid,
                                    "PSM Cert Override Settings Service",
                                    "@mozilla.org/security/certoverride;1",
                                    certoverride_factory);
    */

    // set up some history file (which appears to be
    // required for downloads to work properly, even if we
    // don't store any history entries)

    ns_smartptr<nsIDirectoryService> dir_service = nsGetDirectoryService();
    ns_smartptr<nsIProperties> dir_service_props = dir_service;
   
    ns_smartptr<nsILocalFile> history_file;
    res = NS_NewNativeLocalFile(nsDependentCString((const char*)m_history_filename.mbc_str()), PR_TRUE, &history_file.p);
    if (NS_FAILED(res))
        return false;
       
    res = dir_service_props->Set("UHist", history_file.p);
    if (NS_FAILED(res))
        return false;
   
    // set up a profile directory, which is necessary for many
    // parts of the gecko engine, including ssl on linux

    ns_smartptr<nsILocalFile> prof_dir;
    res = NS_NewNativeLocalFile(nsDependentCString((const char*)m_storage_path.mbc_str()), PR_TRUE, &prof_dir.p);
    if (NS_FAILED(res))
        return false;

    res = dir_service_props->Set("ProfD", prof_dir.p);
    if (NS_FAILED(res))
        return false;
   
   
    // replace the old plugin directory enumerator with our own
    // but keep all the entries that were in there
   
    ns_smartptr<nsISimpleEnumerator> plugin_enum;
    res = dir_service_props->Get("APluginsDL", NS_GET_IID(nsISimpleEnumerator), (void**)&plugin_enum.p);
    if (NS_FAILED(res) || !plugin_enum.p)
        return false;
   
    m_plugin_provider->AddPaths(plugin_enum.p);
    res = dir_service->RegisterProvider(m_plugin_provider);
    if (NS_FAILED(res) || !plugin_enum.p)
        return false;
   

    // set up preferences
   
    ns_smartptr<nsIPref> prefs = nsGetPrefService();
    if (!prefs)
        return false;
   
    // this was originally so that we wouldn't have to set
    // up a prompting service.
    prefs->SetBoolPref("security.warn_submit_insecure", PR_FALSE);
   
    // don't store a history
    prefs->SetIntPref("browser.history_expire_days", 0);
   
    // set path for our cache directory
    PRUnichar* temps = wxToUnichar(m_storage_path);
    prefs->SetUnicharPref("browser.cache.disk.parent_directory", temps);
    freeUnichar(temps);


    m_ok = true;
   
    m_is18 = m_appshell.empty() ? false : true;
   
   
    if (m_is18)
    {
        // 24 May 2008 - a bug was discovered; if a web control is not created
        // in about 1 minute of the web engine being initialized, something goes
        // wrong with the message queue, and the web control will only update
        // when the mouse is moved over it-- strange.  I think there must be some
        // thread condition that waits until the first web control is created.
        // In any case, creating a web control here appears to solve the problem;
        // It's destroyed 10 seconds after creation.
       
        wxWebFrame* f = new wxWebFrame(NULL, -1, wxT(""));
        f->SetShouldPreventAppExit(false);
        f->GetWebControl()->OpenURI(wxT("about:blank"));
       
        DelayedWindowDestroy* d = new DelayedWindowDestroy(f, 10);
    }
   
    return true;


The changes to the Init() method simply remove the local object declarations so that the pointers to objects that need to be unregistered are available to the destructor.

A few other non-related items:
- If you use $(WXWIN) as the base path to wxWidgets in the VS project settings, then you don't need to install wxWidgets into wxWebConnect, but can use it from wherever it is located
- Testapp.cpp has been included in the webconnect.lib project (1.1 release); it shouldn't be there

Hans

P.S., Submitting patches would be easier if we could upload files when posting. Having a Trac project (or similar) to submit tickets to would be even better
P.P.S. Sorry for the mixed spaces and tabs. I tried to use spaces instead of tabs, but Visual Studio prefers tabs.