0
votes

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.

1
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.Damien_The_Unbeliever
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?cremor
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 ?Rohit Prakash
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.mccdyl001

1 Answers

1
votes

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.