~DShowCameraDeviceInteral memory leak


#1

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


#2

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]


#3

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
}
...
}

#4

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.


#5

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 ;
}
}


#6

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.


#7

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

looks like double reference count for single pointer, no?


#8

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

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


#9

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
    ...
}

#10

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?


#11

I confirm that both fixes are required


#12

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