0

I've inherited an existing app to maintain and have hit a snag: it won't release the Excel object from memory once it is completed. I've pulled out the relevant code here. It is a method which creates an Excel workbook using COM Interop:

using System.Runtime.InteropServices;
using Excel = Microsoft.Office.Interop.Excel;
...

private void CreateWorkbook(string workingDirectory, string fileName, object[,] cellValues)
{
    Excel.Application excel = null;
    Excel.Workbooks workbooks = null;
    Excel.Workbook workbook = null;
    Excel.Sheets worksheets = null;
    Excel.Worksheet worksheet = null;
    Excel.Range startRange = null;
    Excel.Range endRange = null;
    Excel.Range selectedRange = null;

    try
    {
        excel = new Excel.Application() { DisplayAlerts = false, Visible = false, ScreenUpdating = false, EnableAutoComplete = false };

        // Statement which stops the excel instance exiting:
        if(excel == null)
        {
            MessageBox.Show("Excel could not be started, please check your machine has office 2013 installed.");
            return;
        }

        workbooks = excel.Workbooks;
        workbook = workbooks.Add(Type.Missing);
        worksheets = workbook.Sheets;
        worksheet = (Excel.Worksheet)worksheets[1];            

        startCellRange = (Excel.Range)worksheet.Cells[1,1];
        endCellRange = (Excel.Range)worksheet.Cells[1 + cellValues.GetLength(0), 1 + cellValues.GetLength(1)]; 
        selectedCells = worksheet.Range[startCellRange, endCellRange];
        selectedCells.Value2 = cellValues;

        workbook.SaveAs(workingDirectory + fileName, Excel.XlFileFormat.xlOpenXMLWorkbook, Type.Missing, Type.Missing, Type.Missing, Type.Missing, Excel.XlSaveAsAccessMode.xlExclusive, Type.Missing, Type.Missing, Type.Missing, Type.Missing, Type.Missing);
        workbook.Close(true, Type.Missing, Type.Missing);
        excel.Quit();
    }
    catch(Exception ex)
    {
        try
        {
            // Catch methods should ideally never throw exceptions, so try close the workbook and quit the excel instance in a try/catch block:
            workbook.Close(false, Type.Missing, Type.Missing);
            excel.Quit();
        }
        catch { }
        MessageBox.Show("An error was encountered while trying to create the excel workbook.\n" + ex.Message + (ex.InnerException == null ? "" : "\n" + ex.InnerException.Message));
    }
    finally
    {
        // Now clean up the com interop stuff:
        // ===================================
        if (Marshal.IsComObject(rangeFont)) Marshal.ReleaseComObject(rangeFont);
        if (Marshal.IsComObject(rangeBorders)) Marshal.ReleaseComObject(rangeBorders);
        if (Marshal.IsComObject(selectedRange)) Marshal.ReleaseComObject(selectedRange);
        if (Marshal.IsComObject(endRange)) Marshal.ReleaseComObject(endRange);
        if (Marshal.IsComObject(startRange)) Marshal.ReleaseComObject(startRange);
        if (Marshal.IsComObject(worksheet)) Marshal.ReleaseComObject(worksheet);
        if (Marshal.IsComObject(worksheets)) Marshal.ReleaseComObject(worksheets);
        if (Marshal.IsComObject(workbook)) Marshal.ReleaseComObject(workbook);
        if (Marshal.IsComObject(workbooks)) Marshal.ReleaseComObject(workbooks);
        if (Marshal.IsComObject(excel)) Marshal.ReleaseComObject(excel);
        if (Marshal.IsComObject(excel)) Marshal.FinalReleaseComObject(excel);

        System.Threading.Thread.Sleep(100);
        GC.Collect();
        GC.WaitForPendingFinalizers();
        System.Threading.Thread.Sleep(500);
        GC.Collect();
        GC.WaitForPendingFinalizers();
    }
}  

The method works fine and disposes the excel object if I comment out the if statement just after the excel = new Excel.Application().... But when that if (excel == null) test is included, the excel object does not free up from memory. I've been trying to figure out why and I'm stumped. I know the runtime can automatically put a wrapper around a com object (i.e. when accessing com objects using more than "2 dots"). But just checking whether a com object is equal to null doesn't seem like a logical place for the runtime to do that, if that is indeed what is going on.

So how does one correctly check if a com object is null? There are other places I'd like to do the check, such as inside the catch block when trying to close the workbook and quit Excel. However, I now fear that checking if the excel object == null inside the catch block will make an invisible wrapper, so I've gone with a nested try..catch block. But that doesn't feel right either.

The question is, what is going on, and what is the best practise for checking if your Excel com object instantiated correctly if my method is incorrect.

6
  • I think you've misdiagnosed the issue here. I cannot think of any mechanism by which the particular statement you've singled out would cause the behaviour you're describing. Commented May 22, 2015 at 6:34
  • Also, you might want to read Marshal.ReleaseComObject considered dangerous Commented May 22, 2015 at 6:35
  • 2
    How does this if statement even make sense? You just assigned excel from a constructor call, how could it ever be null? If the constructor call fails, an exception is thrown, isn't it? Commented May 22, 2015 at 6:35
  • Another thing that disguised me that, How does even the CreateWorkBook will be called if the excel interop dll won't be found on the machine. Next thing is, if excel interop will be there, there would never be excel object null and if excel object would be null anyhow, how can a memory be assigned for the excel interop object ? Commented May 22, 2015 at 6:41
  • Yeah, like I said it was inherited code I'm trying to clean up. In production I've removed the if (excel == null) bit entirely already. But I was trying to find out here why a check for (excel == null) would stop the Excel object from freeing up. Commented May 22, 2015 at 6:46

1 Answer 1

1

Try to move the garbage collector calls to a wrapper method. That way you can be sure that all automatically created references are out of scope when you call it.

private void CreateWorkbookWithCleanup(...)
{
    CreateWorkbook(...);

    // Yes, we really want to call those two methods twice to make sure all
    // COM objects AND all RCWs are collected.
    GC.Collect();
    GC.WaitForPendingFinalizers();
    GC.Collect();
    GC.WaitForPendingFinalizers();
}

Also, in my experience, you don't need to call Marshal.ReleaseComObject, everything is released by the garbage collector if you released all your references. The sleeps shouldn't be needed either. GC calls are blocking anyway.

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

7 Comments

I would be recommending their complete removal, not movement to another location. There's no need to worry about "automatically created references" or anything of the sort - the GC (in concert with the JIT) can be quite aggressive about cleaning up even within a method. Eric Lippert has a recent blog post that discusses this (see first myth - the context is finalization but he is discussing when the GC can clean things up)
Right, off to go try these recommendations right now and check if it clears that Excel object. Its so hard to find a working approach to using Excel interop. The dev before me just let the instances of Excel build up - and this app makes a lot of excel workbooks. Not unusual to see five or six "invisible" instances of Excel in task manager!
@Damien_The_Unbeliever Ok, the wrapper method might not help. But you recommend the complere removal of what? The GC calls? I wouldn't remove them, because even if all references are free to be collected/finalized, the GC might not run for quite some time, preventing the Excel from closing even if only a single COM reference stays alive.
A single GC.Collect might be warranted here. But calling it twice and waiting for finalizers (twice!) smacks of voodoo programming. The first Collect should have kicked of everything required to ensure that no-longer referenced COM objects will be released in the near future. There's no reason to wait around for that to happen.
@Cremor, well I'll be - that worked just perfect (no Marshal.ReleaseComObject.. and moving the GC statements into a wrapper. @Damien_The_Unbeliever - we've run into problems opening up the generated workbooks when there were invisible instances of Excel still running. Most likely due to unreleased handles in bad code still pointing to the generated workbook. One of the easiest ways to make sure no handles remain is to do the GC and check that it actually does disappear from Task Manager.
|

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.