Fix possible memory leaks in DirectshowUtil (1 Viewer)

[peterk2007]

Hi guys,

According to MSDN, if one queries for FilterInfo/PinInfo interfaces, the returning pGraph/pFilter should be released when the interface is no longer needed.

I found numerous occasions in DirectshowUtil when this has not been done. So i did. Hope it helps.

Files:
DirectshowUtil.cs, DsUtils.cs of DirectShowLib.dll. (I added a FreeFilterInfo function here, just like the existing FreePinInfo.)

Submitted the fix: Here

cheers,
Peter

 

[ronilse]

Hi,
Have tested this patch but unfortunately it breaks TV in MP 0.2.3 RC3 + latest SVN, seems like it brakes the pre adding of cards MP does when it starts & DVD on stop command (log with TV errors here), maybe you can have a look & update the patch

Regards
Roy

 

[diehard2]

Hi Peter,

I believe the garbage collector will deal with this. In C# when the interface goes out of scope, it "should" be cleaned up.

 

[peterk2007]

Hi ronilse,

Sorry for the delay, I had no testing environment with TV card for a few days, and I did not want to respond without testing. I have tested now, and it works for me. No DVD errrors and no TV playback errors at all.

Checking the log you have submitted, the crash is at a DirectShowLib.dll call, and there is nowhere near a call to Core\DShowNET\Helper\DirectShowUtil.cs - the one that should crash if my patch causes the problem. (Actually, the graph building before finishes successfully.) I did a modification to DirectShowLib.dll tho, but that was very simple: just added a tiny little function there - which is only called from my modified Core\DShowNET\Helper\DirectShowUtil.cs. So that shouldn't break DirectShowLib.dll down.

I could actually produce a similar (not the same, but similar) error by compiling the code, and only replacing the affected dll-s (core.dll and directshowlib.dll). But as soon as I did a full rebuild and made sure every binary had been replaced by the newly built binaries, there was no problem at all, everything worked fine. So here is where i'll be a pain in the neck : Could you please re-test the patch making sure that all binaries are recompiled and replaced? Thank you indeed.

diehard2:
Well, GC "should" take care of releasing. The problem is when? .

Since each RCW (COM object's .NET wrapper) is very tiny, in some scenarios thousands of RCW's could "sit" and wait for finalization for a very long time before the managed heap gets large enough to trigger a GC. This can be problematic, because while an RCW is tiny, the COM "behind" the RCW can hold onto large amounts of unmanaged memory (which is not part of the managed heap, so does not affects when GC gets triggered).

Since MP is often a long term running application, there are scenarios where dozens or even hundreds of initiated directshow playback could take place (leaving loaded filters languishing in memory for long period of time).

I guess the creators of DirectshowLib think it is better to stay on the safe side and prevent things before happening, because they explicitly mention to release COM objects in their readme.rtf. (Section "Releasing memory to avoid leaks (Marshal.FreeCoTaskMem, FreeAMMediaType, FreePinInfo)")
cheers,
Peter

My test environment:
MediaPortal Version: Daily SVN16350 (+ applied the patch with the latest SVN source 16369)
MediaPortal Skin: BlueTwo
Windows Version: XP Prof. SP2 w. updates
CPU Type: AMD X2 4400+
HDD: Samsung 200GB, 250GB, 320GB
Memory: 1GB
Motherboard: Asus M2N-VM DH
Motherboard Chipset: NVidia
Video Card: NVidia 7600GS
Video Card Driver: Forceware 162.18
Sound Card: Onboard AC97
Sound Card AC3: Yes
1. TV Card: HAUPPAUGE MC150
1. TV Card Type: Analog
1. TV Card Driver: Latest
MPEG2 Video Codec: Cybelink PowerDVD7
MPEG2 Audio Codec: ffdshow
Remote: MCE
TV: Samsung CRT, Toshiba Proj.
TV - HTPC Connection: S-Video, DVI

Attached the log too.

 

[ronilse]

Hi,
peterk2007, i don't have that patch anymore, it was added to SVN, compiled & tested & result posted here. This was a complete build & not dll replacement as you think

Regards
Roy