0

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?

LeopardSkinPillBoxHat
  • 28,915
  • 15
  • 75
  • 111
  • WWhat assertions are you getting? Do you only access the object on a single thread or on multiple threads? – Michael Dec 11 '09 at 00:29
  • Also, do you ever delete m_pMyOtherObject or does it just get torn down when its destructor runs? – Michael Dec 11 '09 at 00:30
  • Sounds like you have the global singleton in one of the COM objects you load. I would suggest moving it to your main program instead to have more lifetime control. – AndersK Dec 11 '09 at 01:28
  • @Michael - I don't have the details on the assertions as I haven't been able to reproduce the problem for a while, but I'll try to get this information. Also, I don't delete m_pMyOtherObject, as the CComPtr object should get destroyed when my global singleton object is destroyed. – LeopardSkinPillBoxHat Dec 11 '09 at 02:13

5 Answers5

2

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

Kevin Shea
  • 920
  • 7
  • 12
  • 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 Dec 11 '09 at 02:21
2

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
  • 555,201
  • 31
  • 458
  • 770
2

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
  • 4,853
  • 1
  • 33
  • 35
  • 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 Dec 11 '09 at 23:28
  • Yeah, after all the playing around this was the only way I could get it to crash. – Igor Zevaka Dec 12 '09 at 22:25
1

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
  • 11,211
  • 2
  • 27
  • 38
0

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
  • 74,528
  • 26
  • 112
  • 128
  • 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 Dec 11 '09 at 02:19
  • 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 Dec 11 '09 at 02:28
  • Oh, it is CComPtr from your comments udner the question. – Igor Zevaka Dec 11 '09 at 02:29