~DShowCameraDeviceInteral memory leak

Hello!
It seems that ~DShowCameraDeviceInteral() doesn’t free its callback declared as

ComSmartPtr callback;
defined as callback = new GrabberCallback (*this);
and released as callback = nullptr;

The memory stays allocated according to visual studio 2010 express report

Detected memory leaks!
Dumping objects ->
{2125} normal block at 0x03482DA0, 12 bytes long.
Data: <P8 ?H > 50 38 DA 01 02 00 00 00 B0 3F 48 03

Juce version 2.0.28

Hmm. My code seems fine, but maybe the win32 stuff needs to be forced to release the grabber. Does it help if you add this?

[code] ~DShowCameraDeviceInteral()
{
if (mediaControl != nullptr)
mediaControl->Stop();

    removeGraphFromRot();

    for (int i = viewerComps.size(); --i >= 0;)
        viewerComps.getUnchecked(i)->ownerDeleted();

    if (sampleGrabber != nullptr)
        sampleGrabber->SetCallback (nullptr, 0);

[/code]

It doesn’t change anything. Is it ok to use the camera device API like this?

here is my test scenario

class MyClass: public CameraDevice::Listener
{
public:
MyClass(int width, int height)
{
   ...
   camera = CameraDevice::openDevice( 0, 176, 144,  width, height);
   if(camera) camera->addListener( this );
   ...
}
~MyClass()
{
   ...
    if(camera)
    {
      ScopedPointer<CameraDevice> tmp = camera;      
      tmp->removeListener( this );             
    }
}
...
private:
ScopedPointer <CameraDevice> camera;  
void imageReceived (const Image& image)
{
 //some processing
}
...
}

Well, in that case it looks like something in DirectShow is hanging onto that reference and not releasing our object. Can’t think of anything else to suggest…

Your code looks fine, but I don’t think there’s any need for all that stuff in your destructor, as the camera object is about to get deleted anyway.

DShow is not the point. Adding some temporary pointer storage in the following manner resolves the issue. I believe you have some nicer solution

class DShowCameraDeviceInteral
{
GrabberCallback * c;
DShowCameraDeviceInteral(): c(nullptr)
{
c = new GrabberCallback (*this);
callback = c;
//callback = new GrabberCallback (*this);
}

~DShowCameraDeviceInteral()
{
callback = nullptr;
delete c ;
}
}

er… but something still has a reference to that object (we know that because its ref-count is non-zero), so to forcibly delete it will leave a dangling pointer somewhere. That’s not a fix, it’s just a bug waiting to happen!

And yes, DShow is the point, because something is keeping a ref-count to that object, and it’s not my code doing it, so I assume it must be DShow.

ComBaseClassHelperBase() initializes RefCount with 1
assignment callback = new GrabberCallback (*this); makes Refcount==2

looks like double reference count for single pointer, no?

Goddammit… Why does it use 1 as the initial ref-count!??

Thanks! I’ll do some sanity-checking there!

It still leaks in juce 2.0.30

Is the code below reasonable ?

DShowCameraDeviceInteral(...)
{
    ...
    callback = new GrabberCallback (*this);  //RefCount==1
    hr = sampleGrabber->SetCallback (callback, 1); //RefCount==2
    /* decrease ref count to avoid memory leak */
    callback->Release(); //RefCount==1
    ...
}

No, that code is definitely not reasonable! I changed it so that the initial ref count is now 0, not 1.

Perhaps it needs both the new fix and the one I suggested earlier in this thread, i.e. adding the SetCallback (nullptr, 0) call?

I confirm that both fixes are required

Ok, thanks, I’ll add the other one too.