views:

157

answers:

5

Background

The application I am working with has several COM DLLs.

One of the COM DLLs has a global singleton object, which stores pointers to COM interfaces in other DLLs. Because it is a global singleton object, I have employed the lazy initialization idiom because it is possible that the interface I am trying to get a pointer to exists in a DLL which hasn't yet been loaded.

(Side-note: This is especially important when registering a single DLL, as the global objects will be constructed within the regsvr32 process, and I don't want the DLL to attempt to acquire an interface to another DLL during this process.)

For example, my lazy initialization method would do something like this:

CComPtr<IMyOtherObject>&
CGlobalSingleton::
GetMyOtherObject()
{
    // SNIP: Other code removed for clarity...

    if (! m_pMyOtherObject)
    {
        hr = pUnknown->QueryInterface(IID_IMyOtherObject,
            (void**) &m_pMyOtherObject);
    }

    return m_pMyOtherObject;
}

NOTE: m_pMyOtherObject is a member variable of the CComPtr type.

The lazy initialization may not be relevant to my problem here, but I'm including it for completeness.

Problem

What I have noticed is that in some circumstances, I get failed assertions when my application shuts down. However, if I change my code to call QueryInterface() every time I need to access IID_IMyOtherOBject (rather than storing it as a member variable) this prevents the assertions.

This appears to me to be a COM object lifetime issue. My hypothesis is that because I am storing a COM pointer, there needs to be some sort of synchronisation between the destruction of the COM interface that I'm pointing to, and my own pointer to it.

My understanding of the CComPtr class (which I am using) is that it takes away a lot of the headaches of dealing with lifetime issues (i.e. calling AddRef() and Release()). But it doesn't appear to be working in my case.

Can anyone pick what I may be doing wrong?

+2  A: 

You're returning a reference to the smart pointer which might not be increasing the reference count. Sorry, I'd check but it's late here. That's my hunch and it fits what you're describing - look into copy constructors for CComPtr.

Hope that helps,

K

Kev
I am calling GetMyOtherObject() so I can invoke methods in the IMyOtherObject interface. I'm only storing the pointer once in CGlobalSingleton (as a member variable), so there should only be a single reference, right?
LeopardSkinPillBoxHat
+1  A: 

I suspect the problem is in your understanding of the copy / assignment semantics of the CComPtr class; I am not particularly familiar with CComPtr, but in my experience smart pointers tend to not work the way you might expect them to. First you should read the documentation for CComPtr and make sure you understand how it works (it wouldn't hurt to look at the source code, either). You could also try putting some breakpoints in the AddRef() and Release() members of CComPtr to see what happens during and after the call to GetMyOtherObject(), particularly if you are temporarily storing the return value and it goes out of scope.

Luke
A: 

Sounds like m_pMyOtherObject is still alive when you shut down your application. In addition to copy constructor issues m_pMyOtherObject should either be a CComPtr or CGlobalSingleton should call m_pMyOtherObject's Release method upon destruction.

Edited for clarity.

Edit Just did a quick test and didn't encounter any issues using function returning a reference to CComPtr. Whilst this is a bit unusual it didn't cause any reference count issues.

I wanted to expand though on what happens if m_pMyOtherObject is not a smart pointer. In this scenario it will never get released. Let me show you why:

  1. You call QueryInterface on some pointer. It will call AddRef on that object.
  2. You return either CComPtr& CComPtr& or naked interface pointer. That is largely irrelevant. No ref count operations take place (unless you assign the return value to another CComPtr, which will AddRef it. But since that CComPtr will balance it out with a call to Release it doesn't matter).
  3. What you end up with is either 1 call to AddRef and 0 to Release or 2 calls to AddRef and 1 to Release. In other words they are unbalanced and you have a reference leak.

To avoid this you need to structure your program like this:

class CGlobalSingleton{

CComPtr<IMyOtherObject> m_spMyOtherObject;

IMyOtherObject* GetMyOtherObject()
{
    // SNIP: Other code removed for clarity...

    if (! m_spMyOtherObject)
    {
        //pUnknown gets AddRef'ed, but that's OK, m_spMyOtherObject will call release when CGlobalSingleton goes out of scope
        hr = pUnknown->QueryInterface(IID_IMyOtherObject,
            (void**) &m_spMyOtherObject);
    }

    return m_pMyOtherObject;
}
}
Igor Zevaka
I'm not sure what you mean when you say I should "store m_pMyOtherObject". This is a member of my class, so I am storing it. Also, from looking at documentation on the web, the whole purpose of CComPtr is to encapsulate the AddRef() and Release() calls so the client doesn't need to worry about lifetime management and reference counting.
LeopardSkinPillBoxHat
What I meant was i don't know if m_pMyOtherObject is CComPtr. Normally you would prefix smart pointer with `m_sp`, so judging by notation it could be just a plain interface pointer.
Igor Zevaka
Oh, it is CComPtr from your comments udner the question.
Igor Zevaka
+2  A: 

Rather than implementing your own global singleton, look at using the IGlobalInterfaceTable interface instead. It is a singleton that is provided by the OS at the process level. Any of your DLLs can put their COM objects into the table, and the other DLLs can retreive them when needed. All you would need to implement on your part is a way for the DLLs to exchange the table's DWORD cookies with each other.

Remy Lebeau - TeamB
+2  A: 

A wild stab in the dark: Is it possible that CGlobalSingleton could get destroyed after CoUninitialize() is called, under any circumstances? If it were, and m_pMyOtherObject was therefore also destroyed after COM uninitialization, it would be another way of causing the access violation that Igor mentioned.

Phil Booth
This could probably happen if CoUninitialize is called at the end of main, but the pointer is released in its global destructor which runs after main returns.
Michael
Yeah, after all the playing around this was the only way I could get it to crash.
Igor Zevaka