Excel COM object not being released

Question

I am trying to have a clean excel break once I load a user's settings file. I am running into something I can't figure out other than it has something to do with the dictionaries. If I comment out one (or both) of the Dictionary filling brackets, the settings file loads and then releases, but if both are running, the excel app won't release. Am I also tying excel to the dictionaries by sourcing the data from them?

I am sure there are other ways to create global dictionaries, but this is the only way I am confidant with at the moment, but I am willing to learn if something better is out there.

The "dictionary filling code" is the for loops with i & j:

for (int i = 0; i < lastRow - 1; i++)
    {
        string key = settingsSheet.Range["B" + (i + 2)].Value; 
        string value = settingsSheet.Range["A" + (i + 2)].Value; 
        DictionaryLoad.DIC.Add(key, value);
    }

here is the full code:

public Form1()
    {
        InitializeComponent();
        txtFileNamePreface.Enabled = false;
        string fileName = "F:\\Shared\\Projects\\State Assoc Clients\\Data Management\\Download Site\\KeyStats Download Statistics\\Naming Conventions.xls";
        LoadProductName(fileName);
    }

    public static class DictionaryLoad
    {
        public static IDictionary<string, string> DIC;
        public static IDictionary<string, string> DIC2;
        static DictionaryLoad()
        {
            DIC = new Dictionary<string, string>();
            DIC2 = new Dictionary<string, string>();
        }
    }

    private void LoadProductName(string fileName)
    {
        //starting up and defining the Excel references
        Excel.Application excelApp = new Excel.Application(); //excel open here
        Excel.Workbook settingsBook = null;
        Excel.Worksheet settingsSheet = null;
        excelApp.Visible = false;
        excelApp.DisplayAlerts = false;

        settingsBook = excelApp.Workbooks.Open(fileName);
        settingsSheet = settingsBook.Sheets["NamingConventions"]; 

        int lastRow = findFirstBlankRow(settingsSheet, "A1") - 1;
        fillComboBox(cbProductType, lastRow, settingsSheet, "A"); 
        fillComboBox(cbYear, lastRow, settingsSheet, "D");

        int lastRow2 = findFirstBlankRow(settingsSheet, "E1");
        fillComboBox(cbRule, lastRow2, settingsSheet, "E");

        for (int i = 0; i < lastRow - 1; i++)
        {
            string key = settingsSheet.Range["B" + (i + 2)].Value; 
            string value = settingsSheet.Range["A" + (i + 2)].Value; 
            DictionaryLoad.DIC.Add(key, value);
        }

        cbProductName.Items.Clear();
        foreach (KeyValuePair<string, string> entry in DictionaryLoad.DIC)
        {
            if (entry.Value == cbProductType.Text)
            { cbProductName.Items.Add(entry.Key); }
        }
        try { cbProductName.SelectedIndex = 0; }
        catch { }

        for (int j = 0; j < lastRow - 1; j++)
        {
            string key = settingsSheet.Range["B" + (j + 2)].Value; 
            string value = settingsSheet.Range["C" + (j + 2)].Value;
            DictionaryLoad.DIC2.Add(key, value);
        }

        cbRule.SelectedIndex = 0;
        cbYear.Text = DateTime.Now.Year.ToString();
        cbQuarter.SelectedIndex = 0;
        cbMonth.Text = DateTime.Now.ToString("MMMM");
        cbProductType.SelectedIndex = 0;
        string workBookName = excelApp.ActiveWorkbook.FullName;
        txtOutputFolder.Text = Path.GetDirectoryName(workBookName);

        settingsBook.Close();
        excelApp.Quit();
        appCleanup(excelApp);
        appCleanup(settingsBook);
        appCleanup(settingsSheet);
        garbageCleanup();
        Application.Exit();
    }
   public void appCleanup(object application1, object application2 = null, object application3 = null)
    {
        Marshal.ReleaseComObject(application1);
        application1 = null;
    }

    public void garbageCleanup()
    {
        GC.Collect();
        GC.WaitForPendingFinalizers();
    }

@GlenThomas I am sorry, I will add that in an edit - added info for both comments — Darw1n34
– Darw1n34, Commented Sep 14, 2015 at 19:25
@Darw1n34 I think you should release the COM objects in an other order: sheet first, then book, then app. — silkfire
– silkfire, Commented Sep 14, 2015 at 19:29

Govert · Accepted Answer · 2015-09-14 20:41:48Z

2

You never need to call Marshal.ReleaseComObject in this context. The runtime is perfectly able to keep track of COM objects and release them when they are no longer referenced. Calling Marshal.ReleaseComObject is a confusing anti-pattern that sadly even some Microsoft documentation mistakenly suggests.

In your case, this means those calls in appCleanup should be removed, though you might still have to set application1 = null to clear the reference - your code does not should how this variable gets used.
You should call the garbage collector routine twice - you might have cases where the references form a cycle, and the first GC call will break the cycle, but the COM objects might only get properly released on the second call.
Finally, you have to be careful with this kind of code in debug builds. References in a method are artificially kept alive until the end of the method so that they will still be accessible in the debugger. This means your local excelApp variable won't be cleaned up by calling the GC inside that method. To avoid this issue, you might follow a pattern like this:
```
public void LoadProductNameAndCleanup(string fileName)
{
    LoadProductName(fileName);
    garbageCleanup();
    garbageCleanup();
}
```

edited Sep 14, 2015 at 20:41

answered Sep 14, 2015 at 20:25

Govert

17k4 gold badges62 silver badges73 bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

Darw1n34 Over a year ago

That did it. Thank you. You were correct as well, I took the use or the Marshal.ReleaseComObject from MSDN documentary on the issue.

Glen Thomas · Accepted Answer · 2015-09-14 20:03:58Z

1

I think maybe you should use string.Copy, because otherwise your dictionaries are holding a reference to string objects linked to the Excel objects. This will create a new instances of the string objects:

for (int i = 0; i < lastRow - 1; i++)
{
    string key = string.Copy(settingsSheet.Range["B" + (i + 2)].Value); 
    string value = string.Copy(settingsSheet.Range["A" + (i + 2)].Value); 
    DictionaryLoad.DIC.Add(key, value);
}

edited Sep 14, 2015 at 20:03

answered Sep 14, 2015 at 19:54

Glen Thomas

10.8k5 gold badges40 silver badges71 bronze badges

9 Comments

Darw1n34 Over a year ago

I was hoping but I get an error :Use of unassigned local variable, for both key and value meh. Thanks though.

Glen Thomas Over a year ago

@Darw1n34 See updated answer... We'll get there eventually! :) I'm just writing this code in my head because I don't have a test application to edit.

Darw1n34 Over a year ago

ugh so close. it throws this error. Member 'string.Copy(string)' cannot be accessed with an instance reference; qualify it with a type name instead. I appreciate the patience

Glen Thomas Over a year ago

@Darw1n34 Ok, got it this time.

Glen Thomas Over a year ago

And if you remove the code that copies the values to the Dictionary it releases the Excel application?

|

Collectives™ on Stack Overflow

Excel COM object not being released

2 Answers 2

1 Comment

9 Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

1 Comment

9 Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related