14

Here's the (potential) problem:

I create a COM object, and then use a 'foreach' to iterate through each element in a collection it returns. Do I need to release each individual element I iterate through in the collection? (See code below.) If so, I can't think of a way to effectively to release it from within a 'finally' statement, just in case there is an error as the item is being operated upon.

Any suggestions?

private static void doStuff()
{
    ComObjectClass manager = null;

    try
    {
        manager = new ComObjectClass();
        foreach (ComObject item in manager.GetCollectionOfItems())
        {
            Log.Debug(item.Name);
            releaseComObject(item); // <-- Do I need this line?
                                    //     It isn't in a 'finally' block...
                                    //             ...Possible memory leak?
        }
    }
    catch (Exception) { }
    finally
    {
        releaseComObject(manager);
    }
}

private static void releaseComObject(object instance)
{
    if (instance != null)
    {
        try
        {
            System.Runtime.InteropServices.Marshal.ReleaseComObject(instance);
        }
        catch
        {
            /* log potential memory leak */
            Log.Debug("Potential memory leak: Unable to release COM object.");
        }
        finally
        {
            instance = null;
        }
    }
}
Matt
  • 3,676
  • 3
  • 34
  • 38

2 Answers2

14

You should not use a foreach statement with a COM object, as a reference is made behind the scenes to which you have no control over releasing. I would switch to a for loop and make sure you never use two dots with COM objects.

The way this would look would be:

try
{
    manager = new ComObjectClass();
    ComObject comObject = null;
    ComObject[] collectionOfComItems = manager.GetCollectionOfItems();
    try
    {
        for(int i = 0; i < collectionOfComItems.Count; i++)
        {
            comObject = collectionOfComItems[i];
            ReleaseComObject(comObject);
        }
    }            
    finally
    {
        ReleaseComObject(comObject);
    }
}
finally 
{
    ReleaseComObject(manager);
}
Community
  • 1
  • 1
Mark Avenius
  • 13,679
  • 6
  • 42
  • 50
  • Thank you! That's the answer I was looking for, and dreading. – Matt Nov 30 '10 at 20:00
  • That would make the whole loop much slower, though. – nawfal Feb 05 '14 at 16:28
  • @nawfal Why would this make the loop slower? – Mark Avenius Feb 05 '14 at 19:24
  • 1
    Depends on COM object itself, its implementation. When i used to run a for loop for Word and Excel interop automation, I found it much slower than running foreach loop. The indexing may not be really good. Who knows, if its a linear search internally? – nawfal Feb 06 '14 at 04:14
  • 1
    I see; either way, a foreach will cause a memory leak. So even if it is faster up front, it will slow you down later ;-) – Mark Avenius Feb 06 '14 at 13:57
  • Would it be as good to move the `try`/`finally` inside of the loop and remove the redundant `ReleaseComObject`? Is there a concern that `collectionOfComItems.Count` would raise an exception? – Sandy Gifford Sep 19 '14 at 15:22
  • @SandyGifford, Looking back at this four years later, I don't recall, but it looks like I was modeling off the OP's try/finally structure. That said, I don't believe it should make a difference, as `collectionOfComItems.Count ` is just operating on an `Array`, not a COM object. – Mark Avenius Sep 19 '14 at 18:24
  • Thanks for curating all these years later! – Sandy Gifford Sep 19 '14 at 19:22
  • @MarkAvenius one more question again 3 years later from your last comment, doesn't `collectionOfComItems` variable need a `ReleaseComObject`? – Ricky Oct 17 '17 at 03:41
0

Another way is to create your own iterator function :

IEnumerable<ComObject> GetChildItems(this ComObjectClass manager) {
    ComObject comObject = null;

    ComObject[] collectionOfComItems = manager.GetCollectionOfItems();
    for (int i = 0; i < collectionOfComItems.Length; i++) {
        try {
            comObject = collectionOfComItems[i];

            yield return comObject;
        } finally {
            if (comObject != null)
                Marshal.ReleaseComObject(comObject);
        }
    }

    yield break;
}

private static void doStuff() {
    ComObjectClass manager = null;

    try {
        manager = new ComObjectClass();

        foreach (ComObject item in manager.GetChildItems()) {
            Log.Debug(item.Name);
        }
    } finally {
        releaseComObject(manager);
    }
}

I feel this makes your code much more readable, especially if you need to iterate through the child items at multiple times.

Jereck
  • 11
  • 3