c# and excel automation - ending the running instance

Solution 1:

UPDATE (November 2016)

I've just read a convincing argument by Hans Passant that using GC.Collect is actually the right way to go. I no longer work with Office (thank goodness), but if I did I'd probably want to give this another try - it would certainly simplify a lot of the (thousands of lines) of code I wrote trying to do things the "right" way (as I saw it then).

I'll leave my original answer for posterity...

As Mike says in his answer, there is an easy way and a hard way to deal with this. Mike suggests using the easy way because... it's easier. I don't personally believe that's a good enough reason, and I don't believe it's the right way. It smacks of "turn it off and on again" to me.

I have several years experience of developing an Office automation application in .NET, and these COM interop problems plagued me for the first few weeks & months when I first ran into the issue, not least because Microsoft are very coy about admitting there's a problem in the first place, and at the time good advice was hard to find on the web.

I have a way of working that I now use virtually without thinking about it, and it's years since I had a problem. It's still important to be alive to all the hidden objects that you might be creating - and yes, if you miss one, you might have a leak that only becomes apparent much later. But it's no worse than things used to be in the bad old days of malloc/free.

I do think there's something to be said for cleaning up after yourself as you go, rather than at the end. If you're only starting Excel to fill in a few cells, then maybe it doesn't matter - but if you're going to be doing some heavy lifting, then that's a different matter.

Anyway, the technique I use is to use a wrapper class that implements IDisposable, and which in its Dispose method calls ReleaseComObject. That way I can use using statements to ensure that the object is disposed (and the COM object released) as soon as I'm finished with it.

Crucially, it'll get disposed/released even if my function returns early, or there's an Exception, etc. Also, it'll only get disposed/released if it was actually created in the first place - call me a pedant but the suggested code that attempts to release objects that may not actually have been created looks to me like sloppy code. I have a similar objection to using FinalReleaseComObject - you should know how many times you caused the creation of a COM reference, and should therefore be able to release it the same number of times.

A typical snippet of my code might look like this (or it would, if I was using C# v2 and could use generics :-)):

using (ComWrapper<Excel.Application> application = new ComWrapper<Excel.Application>(new Excel.Application()))
{
  try
  {
    using (ComWrapper<Excel.Workbooks> workbooks = new ComWrapper<Excel.Workbooks>(application.ComObject.Workbooks))
    {
      using (ComWrapper<Excel.Workbook> workbook = new ComWrapper<Excel.Workbook>(workbooks.ComObject.Open(...)))
      {
        using (ComWrapper<Excel.Worksheet> worksheet = new ComWrapper<Excel.Worksheet>(workbook.ComObject.ActiveSheet))
        {
          FillTheWorksheet(worksheet);
        }
        // Close the workbook here (see edit 2 below)
      }
    }
  }
  finally
  {
    application.ComObject.Quit();
  }
}

Now, I'm not about to pretend that that isn't wordy, and the indentation caused by object creation can get out of hand if you don't divide stuff into smaller methods. This example is something of a worst case, since all we're doing is creating objects. Normally there's a lot more going on between the braces and the overhead is much less.

Note that as per the example above I would always pass the 'wrapped' objects between methods, never a naked COM object, and it would be the responsibility of the caller to dispose of it (usually with a using statement). Similarly, I would always return a wrapped object, never a naked one, and again it would be the responsibility of the caller to release it. You could use a different protocol, but it's important to have clear rules, just as it was when we used to have to do our own memory management.

The ComWrapper<T> class used here hopefully requires little explanation. It simply stores a reference to the wrapped COM object, and releases it explicitly (using ReleaseComObject) in its Dispose method. The ComObject method simply returns a typed reference to the wrapped COM object.

Hope this helps!

EDIT: I've only now followed the link over to Mike's answer to another question, and I see that another answer to that question there has a link to a wrapper class, much as I suggest above.

Also, with regard to Mike's answer to that other question, I have to say I was very nearly seduced by the "just use GC.Collect" argument. However, I was mainly drawn to that on a false premise; it looked at first glance like there would be no need to worry about the COM references at all. However, as Mike says you do still need to explicitly release the COM objects associated with all your in-scope variables - and so all you've done is reduce rather than remove the need for COM-object management. Personally, I'd rather go the whole hog.

I also note a tendency in lots of answers to write code where everything gets released at the end of a method, in a big block of ReleaseComObject calls. That's all very well if everything works as planned, but I would urge anyone writing serious code to consider what would happen if an exception were thrown, or if the method had several exit points (the code would not be executed, and thus the COM objects would not be released). This is why I favor the use of "wrappers" and usings. It's wordy, but it does make for bulletproof code.

EDIT2: I've updated the code above to indicate where the workbook should be closed with or without saving changes. Here's the code to save changes:

object saveChanges = Excel.XlSaveAction.xlSaveChanges;

workbook.ComObject.Close(saveChanges, Type.Missing, Type.Missing);

...and to not save changes, simply change xlSaveChanges to xlDoNotSaveChanges.

Solution 2:

What is happening is that your call to:

Sheet.Cells[iRowCount, 1] = data["fullname"].ToString();

Is essentially the same as:

Excel.Range cell = Sheet.Cells[iRowCount, 1];
cell.Value = data["fullname"].ToString();

By doing it this way, you can see that you are creating an Excel.Range object, and then assigning a value to it. This way also gives us a named reference to our range variable, the cell variable, that allows us to release it directly if we wanted. So you could clean up your objects one of two ways:

(1) The difficult and ugly way:

while (data.Read())
{
    Excel.Range cell = Sheet.Cells[iRowCount, 1];
    cell.Value = data["fullname"].ToString();
    Marshal.FinalReleaseComObject(cell);

    cell = Sheet.Cells[iRowCount, 2];
    cell.Value = data["brand"].ToString();
    Marshal.FinalReleaseComObject(cell);

    cell = Sheet.Cells[iRowCount, 3];
    cell.Value = data["agency"].ToString();
    Marshal.FinalReleaseComObject(cell);

    // etc...
}

In the above, we are releasing each range object via a call to Marshal.FinalReleaseComObject(cell) as we go along.

(2) The easy and clean way:

Leave your code exactly as you currently have it, and then at the end you can clean up as follows:

GC.Collect();
GC.WaitForPendingFinalizers();

if (wSheet != null)
{
    Marshal.FinalReleaseComObject(wSheet)
}
if (wBook != null)
{
    wBook.Close(false, m_objOpt, m_objOpt);
    Marshal.FinalReleaseComObject(wBook);
}
xl.Quit();
Marshal.FinalReleaseComObject(xl);

In short, your existing code is extremely close. If you just add calls to GC.Collect() and GC.WaitForPendingFinalizers() before your 'NAR' calls, I think it should work for you. (In short, both Jamie's code and Ahmad's code are correct. Jamie's is cleaner, but Ahmad's code is an easier "quick fix" for you because you would only have to add the calls to calls to GC.Collect() and GC.WaitForPendingFinalizers() to your existing code.)

Jamie and Amhad also listed links to the .NET Automation Forum that I participate on (thanks guys!) Here are a couple of related posts that I've made here on StackOverflow :

(1) How to properly clean up Excel interop objects in C#

(2) C# Automate PowerPoint Excel -- PowerPoint does not quit

I hope this helps, Sean...

Mike

Solution 3:

Add the following before your call to xl.Quit():

GC.Collect(); 
GC.WaitForPendingFinalizers(); 

You can also use Marshal.FinalReleaseComObject() in your NAR method instead of ReleaseComObject. ReleaseComObject decrements the reference count by 1 while FinalReleaseComObject releases all references so the count is 0.

So your finally block would look like:

finally
{
    GC.Collect(); 
    GC.WaitForPendingFinalizers(); 

    NAR(wSheet);
    if (wBook != null)
        wBook.Close(false, m_objOpt, m_objOpt);
    NAR(wBook);
    xl.Quit();
    NAR(xl);
}

Updated NAR method:

private void NAR(object o)
{
    try
    {
        System.Runtime.InteropServices.Marshal.FinalReleaseComObject(o);
    }
    catch { }
    finally
    {
        o = null;
    }
}

I had researched this awhile ago and in examples I found usually the GC related calls were at the end after closing the app. However, there's an MVP (Mike Rosenblum) that mentions that it ought to be called in the beginning. I've tried both ways and they've worked. I also tried it without the WaitForPendingFinalizers and it worked although it shouldn't hurt anything. YMMV.

Here are the relevant links by the MVP I mentioned (they're in VB but it's not that different):

• http://www.xtremevbtalk.com/showthread.php?p=1157109#post1157109
• http://www.xtremevbtalk.com/showthread.php?s=bcdea222412c5cbfa7f02cfaf8f7b33f&p=1156479#post1156479

Solution 4:

As others have already covered InterOp i would suggest that if you deal with Excel files with XLSX extension you should use EPPlus which will make your Excel nightmares go away.

Solution 5:

I have just answered this question here:

Killing excel process by its main window hWnd