govert / TestNet5Com

Test .NET 5 COM cleanup
0 stars 0 forks source link

Continuing the discussion #2

Open lauxjpn opened 3 years ago

lauxjpn commented 3 years ago

@govert Thanks for taking the time.

Run the following code in net5.0:

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

namespace TestNet5Com
{
    class Program
    {
        static void Main(string[] args)
        {
            Console.WriteLine("Hello World!");
            DoMyExcelStuffAndCleanup();
            Console.WriteLine("All Done");
        }

        static void DoMyExcelStuffAndCleanup()
        {
            DoMyExcelStuff();

            // Now let the GC clean up (repeat, until no more)
            // do
            // {
            //     GC.Collect();
            //     GC.WaitForPendingFinalizers();
            // }
            // while (Marshal.AreComObjectsAvailableForCleanup());
        }

        static void DoMyExcelStuff()
        {
            Application app = new Application();
            app.Visible = true;
            var wb = app.Workbooks.Add();
            var ws = wb.Worksheets[1] as Worksheet;
            ws.Range["A1"].Value = "Hello";
            ws.Range["A2"].Formula = "=2+2";
            ws.Calculate();
            var result = ws.Range["A2"].Formula;
            wb.Close(false);
            Console.WriteLine($"Result: {result}");
            app.Quit();
        }
    }
}

Watch your processes afterwards. You still got your EXCEL.EXE process running. It should be the case, because finalizers are not running on application exit in net5.0, as they do in net48. Its also a common scenarios to not explicitly call Application.Quit(), but to let the user close the Excel instance on demand, which will lead to the same issue.

If you manually trigger GC in a loop at the end, than this is going to work in most scenarios. However, that is not what you were saying:

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.

The release them when they are no longer referenced part is not true for exiting .NET (Core) applications anymore.

Also, depending on the local COM server you are working with, leaking references until you force GC to collect them, can be expensive or lead to unintended side-effects. (Excel handles leaking child references graciously, when it is being closed via Application.Quit(), but that is not the case for COM servers in general, unless they explicitly implement such a behavior.)

However, since the SO question was particular about Excel, your suggested approach to let references leak and just force GC to take care of them before the the client app exits, is still going to work with .NET (Core) for common Excel scenarios, and probably good enough for most users, I will clarify my comment about it on SO.

govert commented 3 years ago

Hi @lauxjpn

Thank you - that's really interesting to see and surprising to me. I was under the mistaken impression that .NET Framework would leave the process running without the GC clean-up call. That's why I was always saying you need to run the GC before closing, to ensure the COM stuff gets cleaned up.

But when testing now I see exactly what you report:

I agree that my wording on what the GC does is not super clear - I should say "the GC tracks the COM references correctly and releases them when the GC cleans up the last runtime wrapper for the COM object." My point there is just that I don't think .NET ever 'leaks' child references - it correctly tracks all the internal references and will release all of them when the GC is called. If you have an example that I can dig into would be helpful.

One of the reasons why I think the manual Marshal.ReleaseComObject is mostly not reliable or a good idea is that the internal COM references are not always surfaced at the language level. So with manual clean-up it's easier in my experience to get the sequence wrong, and get the dreaded 'COM object that has been separated from its underlying RCW cannot be used.' error.

What I have heard of is of some cases where the COM server fails if the references are released in an unexpected order (which is a bug according to the COM rules, but I guess it happens). Then the GC release might be in an order not expected by the buggy server. In these special cases it might by justified to interfere with the .NET COM management by manually triggering some releases in a know order.

Also, I should say the pattern I'm advocating is not to call the GC often during normal runtime, but only at the end of all (or large chunks of) the COM operations, or at the end before shutdown.

So I mean this pattern:

    DoSomeComWork();
    DoSomeMoreComWork();
    DoSomeComWork();
    // At the end when the program is closing
    GCCleanup();

rather than:

    // BAD PATTERN
    DoSomeComWork();
    GCCleanup();
    DoSomeMoreComWork();
    GCCleanup();
    DoSomeComWork();
    GCCleanup();

In the StackOverflow question there was a form and a button involved, which is confusing this pattern - I would certainly not run the GC after every button press.

In practice one would probably put the GC cleanup in an top-level finally handler before the program exists, or after you know all the COM stuff is done, or a major process (writing out a report via Excel) is done.

lauxjpn commented 3 years ago

One of the reasons why I think the manual Marshal.ReleaseComObject is mostly not reliable or a good idea is that the internal COM references are not always surfaced at the language level. So with manual clean-up it's easier in my experience to get the sequence wrong, and get the dreaded 'COM object that has been separated from its underlying RCW cannot be used.' error.

I agree that for .NET developers unfamiliar with COM (which are probably most), tracking your Marshal.ReleaseComObject() calls can be tricky. It's usually a better idea to use a smart pointer class for those cases if you want/need to release your interfaces, but for many common cases, it will be good enough to let the GC cleanup your references.

BTW, there are still mechanisms available for classes to cleanup on app exit (see AppDomain.ProcessExit Event). It's just finalizers that are not being called anymore.

I became aware of the Marshal.AreComObjectsAvailableForCleanup() method through your code sample, so thanks for that!