There have been a lot of discussion around this and everyone tend to agree that you should always call Delegate.EndInvoke to prevent a memory leak (even Jon Skeet said it!).
I always followed this guideline without questioning, but recently I implemented my own AsyncResult class and saw that the only resource that could leak is the AsyncWaitHandle.
(In fact it doesn't really leak because the native resource used by the WaitHandle is encapsulated in a SafeHandle which has a Finalizer, it will add pressure on the finalize queue of the garbage collector though. Even so, a good implementation of AsyncResult will only initialize the AsyncWaitHandle on demand...)
The best way to know if there is a leak is just to try it:
Action a = delegate { };
while (true)
a.BeginInvoke(null, null);
I ran this for a while and the memory stay between 9-20 MB.
Let's compare with when Delegate.EndInvoke is called:
Action a = delegate { };
while (true)
a.BeginInvoke(ar => a.EndInvoke(ar), null);
With this test, the memory play between 9-30 MG, weird eh? (Probably because it takes a bit longer to execute when there is an AsyncCallback, so there will be more queued delegate in the ThreadPool)
What do you think... "Myth busted"?
P.S. ThreadPool.QueueUserWorkItem is a hundred more efficient than Delegate.BeginInvoke, its better to use it for fire & forget calls.
I did run a little test to call an Action delegate and throw an exception inside it. Then I do make sure that I do not flood the thread pool by maintaining only a specified amount of threads running at one time and fill the thread pool continously when a delete call has ended. Here is the code:
static void Main(string[] args)
{
const int width = 2;
int currentWidth = 0;
int totalCalls = 0;
Action acc = () =>
{
try
{
Interlocked.Increment(ref totalCalls);
Interlocked.Increment(ref currentWidth);
throw new InvalidCastException("test Exception");
}
finally
{
Interlocked.Decrement(ref currentWidth);
}
};
while (true)
{
if (currentWidth < width)
{
for(int i=0;i<width;i++)
acc.BeginInvoke(null, null);
}
if (totalCalls % 1000 == 0)
Console.WriteLine("called {0:N}", totalCalls);
}
}
After letting it run for about 20 minutes and over 30 million BeginInvoke calls later the private byte memory consumption was constant (23 MB) as well as the Handle count. There seems no leak to be present. I have read Jeffry Richters book C# via CLR where he states that there is a memory leak. At least this seems no longer to be true with .NET 3.5 SP1.
Test Environment:
Windows 7 x86
.NET 3.5 SP1
Intel 6600 Dual Core 2.4 GHz
Yours,
Alois Kraus
Whether or not it currently leaks memory is not something you should depend on. The framework team could, in the future, change things in a way that could cause a leak, and because the official policy is "you must call EndInvoke" then it's "by design".
Do you really want to take the chance that your app will suddenly start leaking memory sometime in the future because you chose to rely on observed behavior over documented requirements?
In some situations, BeginInvoke doesn't need EndInvoke (particularly in WinForms window messaging). But, there are definitely situations where this matters - like BeginRead and EndRead for async communication. If you want to do a fire-and-forget BeginWrite, you'll probably end up in serious memory trouble after a while.
So, your one test can't be conclusive. You need to deal with many different types of asynchronous event delegates to deal with your question properly.
Consider the following example, which ran on my machine for a few minutes and reached a working set of 3.5 GB before I decided to kill it.
Action a = delegate { throw new InvalidOperationException(); };
while (true)
a.BeginInvoke(null, null);
NOTE: Make sure to run it without a debugger attached or with "break on exception thrown" and "break on user-unhandled exception" disabled.
EDIT: As Jeff points out, the memory issue here is not a leak, but simply a case of overwhelming the system by queuing work faster than it can be processed. Indeed, the same behaviour can be observed by replacing the throw with any suitably long operation. And the memory usage is bounded if we leave enough time between BeginInvoke calls.
Technically, that leaves the original question unanswered. However, regardless of whether or not it can cause a leak, not calling Delegate.EndInvoke is a bad idea since it can cause exceptions to be ignored.
