views:

2940

answers:

13

I'm attempting Excel automation through C#. I have followed all the instructions from Microsoft on how to go about this, but I'm still struggling to discard the final reference(s) to Excel for it to close and to enable the GC to collect it.

A code sample follows. When I comment out the code block that contains lines similar to:

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

then the file saves and Excel quits. Otherwise the file saves but Excel is left running as a process. The next time this code runs it creates a new instance and they eventually build up.

Any help is appreciated. Thanks.

This is the barebones of my code:

        Excel.Application xl = null;
        Excel._Workbook wBook = null;
        Excel._Worksheet wSheet = null;
        Excel.Range range = null;

        object m_objOpt = System.Reflection.Missing.Value;

        try
        {
            // open the template
            xl = new Excel.Application();
            wBook = (Excel._Workbook)xl.Workbooks.Open(excelTemplatePath + _report.ExcelTemplate, false, false, m_objOpt, m_objOpt, m_objOpt, m_objOpt, m_objOpt, m_objOpt, m_objOpt, m_objOpt, m_objOpt, m_objOpt, m_objOpt, m_objOpt);
            wSheet = (Excel._Worksheet)wBook.ActiveSheet;

            int iRowCount = 2;

            // enumerate and drop the values straight into the Excel file
            while (data.Read())
            {

                wSheet.Cells[iRowCount, 1] = data["fullname"].ToString();
                wSheet.Cells[iRowCount, 2] = data["brand"].ToString();
                wSheet.Cells[iRowCount, 3] = data["agency"].ToString();
                wSheet.Cells[iRowCount, 4] = data["advertiser"].ToString();
                wSheet.Cells[iRowCount, 5] = data["product"].ToString();
                wSheet.Cells[iRowCount, 6] = data["comment"].ToString();
                wSheet.Cells[iRowCount, 7] = data["brief"].ToString();
                wSheet.Cells[iRowCount, 8] = data["responseDate"].ToString();
                wSheet.Cells[iRowCount, 9] = data["share"].ToString();
                wSheet.Cells[iRowCount, 10] = data["status"].ToString();
                wSheet.Cells[iRowCount, 11] = data["startDate"].ToString();
                wSheet.Cells[iRowCount, 12] = data["value"].ToString();

                iRowCount++;
            }

            DirectoryInfo saveTo = Directory.CreateDirectory(excelTemplatePath + _report.FolderGuid.ToString() + "\\");
            _report.ReportLocation = saveTo.FullName + _report.ExcelTemplate;
            wBook.Close(true, _report.ReportLocation, m_objOpt);
            wBook = null;

        }
        catch (Exception ex)
        {
            LogException.HandleException(ex);
        }
        finally
        {
            NAR(wSheet);
            if (wBook != null)
                wBook.Close(false, m_objOpt, m_objOpt);
            NAR(wBook);
            xl.Quit();
            NAR(xl);
            GC.Collect();
        }

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


Update

No matter what I try, the 'clean' method or the 'ugly' method (see answers below), the excel instance still hangs around as soon as this line is hit:

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

If I comment that line out (and the other similar ones below it, obviously) the Excel app exits gracefully. As soon as one line per above is uncommented, Excel sticks around.

I think I'm going to have to check if there's a running instance prior to assigning the xl variable and hook into that instead. I forgot to mention that this is a windows service, but that shouldn't matter, should it?


+2  A: 

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):

Ahmad Mageed
Using the garbage collector in this way should not be necessary. I have an extremely complex Office automation system written in .NET and I will admit that in one case I did have to resort to doing this, but for any small-to-medium project you ought to be able to clean up the COM references so that it's not necessary.Note that if you do use this method, you need to do the Collect/Wait twice, since the first iteration simply marks stuff as available for collection.
Gary McGill
Hi Gary, I agree that you do not *have* to do it this way, but as a practical matter I find that it is too easy to miss one (or more) explicit Marshal.FinalReleaseCom() object calls and then you will hang. Finding your error in a medium to large scale project can be next to impossible. (Neither the compiler nor the code analysis tools will help you.) So I find it far safer to use the "final cleanup" approach.
Mike Rosenblum
Gary, you do not have to call GC.Collect() and GC.WaitForPendingFinalizers() twice. What happens is, when you call GC.Collect() the finalizers for the RCWs are all called, which decrements the COM reference count and releases the reference on the COM side of the fence (which is all you care about here). The RCW itself, however, will remain uncollected until the NEXT garbage collection. But we don't care about that -- the RCW has already dropped it's payload. (If one is using VSTO, however, you DO have to call Collect/Wait twice, because VSTO uses an additional layer of finalizers.)
Mike Rosenblum
Mike, that's interesting about GC x 2 not being required outside of VSTO, thanks.
Gary McGill
Yep, the double call was new to me too, good to know.
Ahmad Mageed
Yeah, I've never seen that stated anywhere, but if you understand how finalizers work, and how the RCW is calling Marshal.ReleaseComObject() in the finalizer, then the fact that the RCW is promoted doesn't really matter... So I always wondered why all code examples show it being called 2x. And the answer is that it is needed for VSTO. Here's the only source I can find on this at the moment: http://social.msdn.microsoft.com/forums/en-US/vsto/thread/377fbebc-021d-4e48-aaef-d4658cc221b3/.
Mike Rosenblum
That doesn't seem like a wildly credible source for your argument re calling GC.Collect twice, since while the article says that you need it for VSTO, it does not imply that it's *only* needed for VSTO. The article at http://msdn.microsoft.com/en-us/library/aa679807(office.11).aspx is not a VSTO article, but *does* suggest a double call - and there seem to be far more articles in favor of doing so than against. However, thinking through it I do agree - the 2nd Collect call will free up the managed RCW object but will have no bearing on the COM reference count, so is not needed. Thanks again.
Gary McGill
That source on VSTO actually does imply that it's only needed for VSTO because it explains *why* it is needed for VSTO -- a reason that does not exist when VSTO is not present. "However, thinking through it I do agree." Yes, fully understanding this comes from thinking through how garbage collection works when finalizers are involved: the object are not collected until the *next* garbage collection, but the finalizers are all called during the *current* garbage collection, which is what we care about with a RCW. 'GC.Collect' and 'GC.WaitForPendingFinalizers' only have to be called *once*.
Mike Rosenblum
A: 

What I ended up doing to solve a similar problem was get the process Id and kill that as a last resort...

[DllImport("user32.dll", SetLastError = true)]
   static extern IntPtr GetWindowThreadProcessId(int hWnd, out IntPtr lpdwProcessId);

...

objApp = new Excel.Application();

IntPtr processID;
GetWindowThreadProcessId(objApp.Hwnd, out processID);
excel = Process.GetProcessById(processID.ToInt32());

...

objApp.Application.Quit();
Marshal.FinalReleaseComObject(objApp);
_excel.Kill();
Andy Mikula
Hi Andy,I resorted to giving your method a go. However, even though the Excel instance is killed, I'm still left with an Excel file that thinks it's being accessed by another application. I get prompted for the Notify/Read Only message when I open the file...
Sean
Oh dear lord, don't do that (kill the process). Cleaning up your COM references is a pain, but very do-able. Terminating the process is like shooting your unloved girlfriend to avoid the trouble of breaking up with her.
Gary McGill
Gary - do-able perhaps, but in my case not at all reliable. All of the other methods left me with processes running (20-30 of them) after an hour or so of execution.
Andy Mikula
A: 

Here's the contents of my hasn't-failed-yet finally block for cleaning up Excel automation. My application leaves Excel open so there's no Quit call. The reference in the comment was my source.

finally
{
    // Cleanup -- See http://www.xtremevbtalk.com/showthread.php?t=160433
    GC.Collect();
    GC.WaitForPendingFinalizers();
    GC.Collect();
    GC.WaitForPendingFinalizers();
    // Calls are needed to avoid memory leak
    Marshal.FinalReleaseComObject(sheet);
    Marshal.FinalReleaseComObject(book);
    Marshal.FinalReleaseComObject(excel);
}
Jamie Ide
+3  A: 

Hi Sean,

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

Mike Rosenblum
Straight from the source :)
Ahmad Mageed
lol, thanks for the plug, Ahmad. :-)
Mike Rosenblum
Excellent, thanks - I attempted a search on stackoverflow for this. Obviously I didn't phrase my search well enough to find your posts, Mike. Thanks for taking the time to re-explain. I understand about the hidden references and why it wasn't being cleaned up, but I could not find a sample that showed the 'ugly' way on the web. I'll try this all out this evening. I'll mark as Answered at that point. (I'm in Ireland so time differences play havoc!). Thanks to all who also helped out, above.
Sean
Sean, good luck with it, and let us know how it goes. I posted the "ugly way" only to explain what is going on "under the hood". You can use it as an experiment to help you understand how it all works, but I would not use it for actual production code. The reason is that it is too easy to make one tiny mistake somewhere in your code, and Excel will hang. Neither the compiler nor any code analysis program will be able to find this error for you. This approach also bloats your code with lots of calls to Marshal.FinalReleaseComObject(), distracting from the intent of one's code. (Continued...)
Mike Rosenblum
... Therefore, as a practical matter, I do believe that the way to go is for the final cleanup section of one's code to call GC.Collect() and GC.WaitForPendingFinaliers, followed by the release of all the named variable references using Marshal.FinalReleaseComObject() on each reference, one by one.
Mike Rosenblum
Hi Mike,Unfortunately I can't get this to work. I have added an update to my original post but in case you never saw that, I am now thinking of only creating a new instance of Excel if there is no running instance to hook into. This will be running as a windows service and Excel will be installed on the server purely for this app so shouldn't have an adverse affect on other Excel users (as there won't be any). Thanks for the pointers though - it's got me stumped.
Sean
Is this application designed to support a web-site or the like, server-side? If so, this can be highly problematic. (See: http://support.microsoft.com/default.aspx?scid=kb;EN-US;257757.) You can use VSTO for server-side automation, or consider using a 3rd party program for this. Excel, natively, is not well suited for this. But you may be fine -- I don't know. As for the hanging issue, I would post your updated code and hopefully we'll see an improvement to be made. If not, you can resort to Process.Kill (see: http://www.xtremevbtalk.com/showthread.php?p=1326018#post1326018).
Mike Rosenblum
Hi again Mike and thanks for the comment. The service will reside on a machine that's not an IIS server, though calls will be made to it from an IIS server. I also tried the Process.Kill method which worked, but I could then not delete the file after emailing it as it was 'still in use'. When I attempted to open it, it gave me the Notify/Read Only prompt - unfortunately at that time it was time for bed... I'm at work now and will look again later this evening. I've also just seen http://support.microsoft.com/kb/306023 and will dig into that a bit.
Sean
Can I point out that the "xl.Workbooks.Open" call in the OP's code is also a source of a leak. There's an intermediate Workbooks object there that needs to be released. I disagree with Mike on avoiding the "ugly" way. It requires discipline, but it can be made to work, and can also be made less ugly (see my answer for details).
Gary McGill
Hi Gary - did you post an answer? (Apologies if you're still writing it...)
Sean
Hi Gary, I totally agree that the "ugly" way can be made to work. However, in a large project, the probability of missing something, somewhere, will grow, and one's ability to find such an error (due to the size of the project) will shrink. It becomes a totally unmanageable approach past a certain size -- and not a very big size either, I'm afraid.
Mike Rosenblum
Sean, you need to show your updated code, including the Process.Kill call. If you call Workbook.Close and then Quit correctly (or call Process.Kill, if you have to) there is no way that this should be happening. Show your code, and hopefully we can see something else that needs to be corrected. As it is now, we are just guessing. (Your code is small enough, currently, to get the "ugly way" to work, by the way. So Gary it right, at least at its current size. But if you go that way, then show that code!)
Mike Rosenblum
(If you go the "ugly way, by the way, you need to reference the 'Workbooks' collection by name and release it explicitly when done. This is what Gary was referring to. If you go with the GC.Collect / GC.WaitForPendingFinilizers approach, instead, you don't need to bother with it, but if you use the explicit approach, you do.)
Mike Rosenblum
Ok - will do when I get back home this evening. Thanks for persevering.
Sean
A: 

Have you considered using a pure .NET solution such as SpreadsheetGear for .NET? Here is what your code might like like using SpreadsheetGear:

// open the template            
using (IWorkbookSet workbookSet = SpreadsheetGear.Factory.GetWorkbookSet())
{
    IWorkbook wBook = workbookSet.Workbooks.Open(excelTemplatePath + _report.ExcelTemplate);
    IWorksheet wSheet = wBook.ActiveWorksheet;
    int iRowCount = 2;
    // enumerate and drop the values straight into the Excel file            
    while (data.Read())
    {
        wSheet.Cells[iRowCount, 1].Value = data["fullname"].ToString();
        wSheet.Cells[iRowCount, 2].Value = data["brand"].ToString();
        wSheet.Cells[iRowCount, 3].Value = data["agency"].ToString();
        wSheet.Cells[iRowCount, 4].Value = data["advertiser"].ToString();
        wSheet.Cells[iRowCount, 5].Value = data["product"].ToString();
        wSheet.Cells[iRowCount, 6].Value = data["comment"].ToString();
        wSheet.Cells[iRowCount, 7].Value = data["brief"].ToString();
        wSheet.Cells[iRowCount, 8].Value = data["responseDate"].ToString();
        wSheet.Cells[iRowCount, 9].Value = data["share"].ToString();
        wSheet.Cells[iRowCount, 10].Value = data["status"].ToString();
        wSheet.Cells[iRowCount, 11].Value = data["startDate"].ToString();
        wSheet.Cells[iRowCount, 12].Value = data["value"].ToString();
        iRowCount++;
    }
    DirectoryInfo saveTo = Directory.CreateDirectory(excelTemplatePath + _report.FolderGuid.ToString() + "\\");
    _report.ReportLocation = saveTo.FullName + _report.ExcelTemplate;
    wBook.SaveAs(_report.ReportLocation, FileFormat.OpenXMLWorkbook);
}

If you have more than a few rows, you might be shocked at how much faster it runs. And you will never have to worry about a hanging instance of Excel.

You can download the free trial here and try it for yourself.

Disclaimer: I own SpreadsheetGear LLC

Joe Erickson
Thanks Joe - if I get a lot more Excel work and/or a higher paying client I might spend the time evaluating it. At this point in time though, this is a small project and the route I am taking seems the most viable. I'll bear it in mind though.
Sean
+5  A: 

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.

Gary McGill
Very nice Gary, I like this a lot. As a compromise, one could get away with only a single wrapper for the Excel.Application instance only. Then within the Dispose routine for the Excel.Application wrapper, one would use the Process.Kill approach that gets the process id using Application.Hwnd and GetWindowThreadProcessId (http://stackoverflow.com/questions/51462/killing-excelexe-on-server#312513). This is essentially what I do for my production code and have had zero problems. It saves a ton of nesting that you are showing, so the code flows much more naturally.
Mike Rosenblum
I've also never had a problem using the GC.Collect() and GC.WaitForPendingFinalizers() approach, and I've helped countless coders on the <a href="http://www.xtremevbtalk.com/forumdisplay.php?f=105">XVBT .NET Automation</a> forum. This one might be a first though... :-(
Mike Rosenblum
Mike, while killing a process like that may work 99.99% of the time, sooner or later it'll get killed when it's in the middle of doing something that shouldn't be left half-done. For example, if it were Word rather than Excel, it might be writing changes to Normal.dot... Killing a process also gives it no opportunity to clean up after itself; we had an example of that in this very question-thread, with a file being left read-only after Excel was killed.
Gary McGill
Indeed, which is actually why I asked him to show his code. That absolutely should not be happening and does not have anything to do with a "hanging instance". The final cleanup section of the code needs to check if the 'Workbook' is null, if not, close it, then call Marshall.FinalReleaseComObject(), then call Excel.Application.Quit() and then Process.Kill. This brings you up from 99.99% to 100%. But we'd need to see his code.
Mike Rosenblum
Thanks both for the great discussion. I'm drawn by the wrapper idea (also good for code re-use) and will attempt it over the weekend. I will post my code this evening if I get a chance, though I suspect weekends are somewhat quiter on StackOverflow - or rather, they should be :)
Sean
quieter... not quiter...
Sean
Gary, I must say I love your pattern. It takes some time to implement it thorugh the application, but I think its woth it. One problem I have is, that I read from my Excel object to create a SQL script. Once its finished and wants to quit/release the excel object, I get a warning message, if I would like to save my changes. But I havent changed anything in the excel object. I dont get it. In either way, how can I suppress this or reject my "changes" so that the object quits/releases automatically without forcing me to click no on dialogbox? Your answer is highly appreciated, Regards, Kave
Kave
@Kave: you'll get a warning at Quit if there are unsaved changes in any open workbooks. It doesn't sound ideal that you get to the end and still have workbooks open - you should probably explicitly close them. When you close them, pass true as the SaveChanges optional parameter to avoid a warning message.
Gary McGill
Thanks Gary for getting back to me. Since I am generating a SQL script I don't want really to change or save the original excel file. But I understand that the created workbooks in memory referencing the excel file might have changes, which popup at the end of the final {application.ComObject.Quit();} In order to fix this you suggest to quit the workbooks, correct? But shouldnt they quit automatically because of public void Dispose() { Marshal.ReleaseComObject(_comObject); }If you may give me a code sample how to do this according to the pattern many thanks
Kave
@Kave: releasing the *reference* to the COM object won't affect the underlying object. If you want the workbook to close, you need to call its Close method. I'll update my example code to show the workbook being closed.
Gary McGill
Gary, thank you very much. It works now like a charm. Only a slight mistake in your new code "Type.Missing" should be "Missing.Value" I think. ;) Cheers
Kave
A: 

I think you are giving Excel references back to your objects:

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

is creating a reference to your string object, change it to:

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

now it has copied the value, rather then a reference

FigmentEngine
When I have enough reputation to do so, I will come back and down-vote this answer. It's imaginative, I'll give you that.
Gary McGill
A: 

The easiest way to paste code is through a question - this doesn't mean that I have answered my own question (unfortunately). Apologies to those trying to help me - I was not able to get back to this until now. It still has me stumped... I have completely isolated the Excel code into one function as per

private bool GenerateDailyProposalsReport(ScheduledReport report)
{
    // start of test

    Excel.Application xl = null;
    Excel._Workbook wBook = null;
    Excel._Worksheet wSheet = null;
    Excel.Range xlrange = null;
    object m_objOpt = System.Reflection.Missing.Value;

    xl = new Excel.Application();
    wBook = (Excel._Workbook)xl.Workbooks.Open(@"E:\Development\Romain\APN\SalesLinkReportManager\ExcelTemplates\DailyProposalReport.xls", false, false, m_objOpt, m_objOpt, m_objOpt, m_objOpt, m_objOpt, m_objOpt, m_objOpt, m_objOpt, m_objOpt, m_objOpt, m_objOpt, m_objOpt);
    wSheet = (Excel._Worksheet)wBook.ActiveSheet;
    xlrange = wSheet.Cells[2, 1] as Excel.Range;

    // PROBLEM LINE ************
    xlrange.Value2 = "fullname";
    //**************************

    wBook.Close(true, @"c:\temp\DailyProposalReport.xls", m_objOpt);
    xl.Quit();

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

    Marshal.FinalReleaseComObject(xlrange);
    Marshal.FinalReleaseComObject(wSheet);
    Marshal.FinalReleaseComObject(wBook);
    Marshal.FinalReleaseComObject(xl);

    xlrange = null;
    wSheet = null;
    wBook = null;
    xl = null;

    // end of test
    return true;

}

If I comment out the PROBLEM LINE above, the instance of Excel is released from memory. As it stands, it does not. I'd appreciate any further help on this as time is fleeting and a deadline looms (don't they all).

Please ask if you need more information. Thanks in anticipation.

Addendum

A bit more information that may or may not shed more light on this. I have resorted to killing the process (stopgap measure) after a certain time lapse (5-10 seconds to give Excel time to finish it's processes). I have two reports scheduled - the first report is created and saved to disk and the Excel process is killed, then emailed. The second is created, saved to disk, the process is killed but suddenly there is an error when attempting the email. The error is: The process cannot access the file'....' etc

So even when the Excel app has been killed, the actual Excel file is still being held by the windows service. I have to kill the service to delete the file...

Sean
A: 

I'm afraid I am running out of ideas here Sean. :-(

Gary could have some thoughts, but although his wrapper approach is very solid, it won't actually help you in this case because you are already doing everything pretty much correctly.

I'll list a few thoughts here. I don't see how any of them will actually work because your mystery line

xlrange.Value2 = "fullname";

would not seem to be impacted by any of these ideas, but here goes:

(1) Don't make use of the _Workbook and _Worksheet interfaces. Use Workbook and Worksheet instead. (For more on this see: Excel interop: _Worksheet or Worksheet?.)

(2) Any time you have two dots (".") on the same line when accessing an Excel object, break it up into two lines, assigning each object to a named variable. Then, within the cleanup section of your code, explicitly release each variable using Marshal.FinalReleaseComObject().

For example, your code here:

 wBook = (Excel._Workbook)xl.Workbooks.Open(@"E:\Development\Romain\APN\SalesLinkReportManager\ExcelTemplates\DailyProposalReport.xls", false, false, m_objOpt, m_objOpt, m_objOpt, m_objOpt, m_objOpt, m_objOpt, m_objOpt, m_objOpt, m_objOpt, m_objOpt, m_objOpt, m_objOpt);

could be broken up to:

Excel.Workbooks wBooks = xl.Workbooks;
wBook = wBooks.Open("@"E:\Development\...\DailyProposalReport.xls", etc...);

And then later, within the cleanup section, you would have:

Marshal.FinalReleaseComObject(xlrange);
Marshal.FinalReleaseComObject(wSheet);
Marshal.FinalReleaseComObject(wBook);
Marshal.FinalReleaseComObject(wBooks); // <-- Added
Marshal.FinalReleaseComObject(xl);

(3) I am not sure what is going on with your Process.Kill approach. If you call wBook.Close() and then xl.Quit() before calling Process.Kill(), you should have no troubles. Workbook.Close() does not return execution to you until the workbook is closed, and Excel.Quit() will not return execution until Excel has finished shutting down (although it might still be hanging).

After calling Process.Kill(), you can check the Process.HasExited property in a loop, or, better, call the Process.WaitForExit() method which will pause until it has exited for you. I would guess that this will generally take well under a second to occur. It is better to wait less time and be certain than to wait 5 - 10 seconds and only be guessing.

(4) You should try these cleanup ideas that I've listed above, but I am starting to suspect that you might have an issue with other processes that might be working with Excel, such as an add-in or anti-virus program. These add-ins can cause Excel to hang if they are not done correctly. If this occurs, it can be very difficult or impossible to get Excel to release. You would need to figure out the offending program and then disable it. Another possibility is that operating as a Windows Service somehow is an issue. I don't see why it would be, but I do not have experience automating Excel via a Windows Service, so I can't say. If your problems are related to this, then using Process.Kill will likely be your only resort here.

This is all I can think of off-hand, Sean. I hope this helps. Let us know how it goes...

-- Mike

Mike Rosenblum
Again Mike, thanks for the comprehensive answer and help. With regards to 1) and 2) above - at this stage I have tried all of these and many other permutations trying to get this solved! I have even spawned a new thread to handle the Excel work in case there was some underlying issue there, but no joy. What struck me as odd was that it was the actual windowsService.exe that keeps a handle on the file, not allowing me to delete it until I quit the service itself. I might be resorting to Excel's xml to get this done now. I will try the suggestions above first though. Thanks again.
Sean
Further to 3) above, when debugging the code the wBook.Close and xl.Quit return quickly and without error. After that the process.kill is called, but the file is still left in an open state. I can open the file but get prompted that the file is already open - and I have to choose Read Only or Notify even after killing the orphaned Excel.exe. Stranger than fiction - maybe I need a re-write in case it's something obvious and I've just been looking at it too long. Sigh.
Sean
Ugh... So sorry to be hearing about all this, Sean. It's not *suposed* to be this hard. But it does happen. :-(
Mike Rosenblum
A: 

Sean,

I'm going to re-post your code again with my changes (below). I've avoided changing your code too much, so I haven't added any exception handling, etc. This code is not robust.

private bool GenerateDailyProposalsReport(ScheduledReport report)
{
    Excel.Application xl = null;
    Excel.Workbooks wBooks = null;
    Excel.Workbook wBook = null;
    Excel.Worksheet wSheet = null;
    Excel.Range xlrange = null;
    Excel.Range xlcell = null;

    xl = new Excel.Application();

    wBooks = xl.Workbooks;

    wBook = wBooks.Open(@"DailyProposalReport.xls", false, false, Type.Missing, Type.Missing, Type.Missing, Type.Missing, Type.Missing, Type.Missing, Type.Missing, Type.Missing, Type.Missing, Type.Missing, Type.Missing, Type.Missing);

    wSheet = wBook.ActiveSheet;

    xlrange = wSheet.Cells;

    xlcell = xlrange[2, 1] as Excel.Range;

    xlcell.Value2 = "fullname";

    Marshal.ReleaseComObject(xlcell);
    Marshal.ReleaseComObject(xlrange);
    Marshal.ReleaseComObject(wSheet);

    wBook.Close(true, @"c:\temp\DailyProposalReport.xls", Type.Missing);

    Marshal.ReleaseComObject(wBook);
    Marshal.ReleaseComObject(wBooks);

    xl.Quit();

    Marshal.ReleaseComObject(xl);

    return true;
}

Points to note:

  1. The Workbooks method of the Application class creates a Workbooks object which holds a reference to the corresponding COM object, so we need to ensure we subsequently release that reference, which is why I added the variable wBooks and the corresponding call to ReleaseComObject.

  2. Similarly, the Cells method of the Worksheet object returns a Range object with another COM reference, so we need to clean that up too. Hence the need for 2 separate Range variables.

  3. I've released the COM references (using ReleaseComObject) as soon as they're no longer needed, which I think is good practice even if it isn't strictly necessary. Also, (and this may be superstition) I've released all the objects owned by the workbook before closing the workbook, and released the workbook before closing Excel.

  4. I'm not calling GC.Collect etc. because it shouldn't be necessary. Really!

  5. I'm using ReleaseComObject rather than FinalReleaseComObject, because it should be perfectly sufficient.

  6. I'm not null-ing the variables after use; once again, it's not doing anything worthwhile.

  7. Not relevant here, but I'm using Type.Missing instead of System.Reflection.Missing.Value for convenience. Roll on C#v4 where optional parameters will be supported by the compiler!

I've not been able to compile or run this code, but I'm pretty confident it'll work. Good luck!

Gary McGill
Hi Gary - thanks for that, the code was fine except for one missing cast. Anyhow, the code ran but still did not clean up Excel after itself. I've lost the will to continue the good fight and I'm going to have to resort to Open Xml to create and save the xls file. As MS says: Microsoft does not recommend or support server-side Automation of Office. Now I see why! It has to be something to do with it being run through a windows service.Thanks to all for the help - especially Mike and Gary. It's great to know there are folk willing to take the time to aid developers.
Sean
Nice list of points Gary, and nice pickup on the 'Cells' method, that could have been the ticket... The weird thing is that Seans code hangs not from the line 'xlrange = wSheet.Cells[2, 1] as Excel.Range', but from adding the line 'xlrange.Value2 = "fullname"', which does not make any sense.
Mike Rosenblum
Sean, I guess it could be due to working through the Windows Service, I don't see why this could be, but I don't have experience with this, so I don't know. It could also be an add-in running withing Excel or anti-virus or the like that is causing Excel to hang -- I have seen this myself. Whatever it is, it is something odd... Sorry it worked out this way for you. :-(
Mike Rosenblum
Sean, my application runs as a Windows Service with no problems, so while it's certainly an extra complication I wouldn't consider that to be a show-stopper. I'm sorry that we couldn't get this to work. I'd be prepared to stick with it a little longer, but I can quite understand that you'll be fed up with it by now.
Gary McGill
Thanks for testing that Gary. In that case you've taken away the last possible piece of straw that I had to cling on to :)I've started on the route of using XML as it's only a couple of reports with minimal formatting that I need to create. If I get a chance to re-investigate this at a later date I will. Thanks again for the help.Just as an aside, I am using the 2007 PIAs on Windows XP - I pesume you guys are also on 2007 with no problems...
Sean
A: 

I have just answered this question here:

Killing excel process by its main window hWnd

nightcoder
A: 

May be my article Excel sheet processing helps somebody to get excel automation application ready fast. There is described few tricks.

Universal
A: 

There's no need to use the excel com objects from C#. You can use OleDb to modify the sheets.

http://www.codeproject.com/KB/office/excel_using_oledb.aspx

jgauffin