Does this Resharper fix for disposed closure warning make any sense?

Question

I'm working on getting rid of some warnings from a static code analysis. In one specific case there was no disposing being done on a ManualResetEvent.

The code in question executes a Func on the main thread and blocks the calling thread for a certain number of milliseconds. I realize this sounds like a weird thing to do, but it's outside the scope of this question, so bear with me.

Suppose I add a using statement like so:

object result = null;
using (var completedEvent = new ManualResetEvent(false))
{
    _dispatcher.BeginInvoke((Action)(() =>
        {
            result = someFunc;
            completedEvent.Set();  // Here be dragons!
        }));

    completedEvent.WaitOne(timeoutMilliseconds);
    return result;
}

Now, I realize that this is likely to cause problems. I also happen to use Resharper and it warns me with the message "Access to disposed closure".

Resharper proposes to fix this by changing the offending line to:

if (completedEvent != null)
{ 
    completedEvent.Set();
}

Now, the proposed solution puzzles me. Under normal circumstances, there would be no reason why a variable would be set to null by a using statement. Is there some implementation detail to closures in .NET that would guarantee the variable to be null after the variable that has been closed over is disposed?

As a bonus question, what would be a good solution to the problem of disposing the ManualResetEvent?

Answer 1

You are mixing up ReSharper's "quick fix" and "context action". When ReSharper proposes to fix something, then most likely you'd see a bulb there. You don't see a bulb here, because there are no quick fixes to this warning.

But aside from quick fixes ReSharper also has "context actions", where it can do some routine tasks for you (think of them like a small refactorings). When ReSharper has a context action for a code under cursor, it would show you a pick. Here you see a context action called "Check if something is not null". It has no relation to a warning, and there is no convention that after disposing a variable would be set to null.

Also, when you press Alt-Enter, you'd see a striked-out bulb to give you impression that ReSharper doesn't suggest any quick fixes for this warning, but it can disable it with comments. In fact, that's the only way to make this warning go away easily. But I'd rewrite this piece of code instead.

Answer 2

I ran into exactly this only a few hours ago.

It's a false alarm. R# doesn't understand that execution will block until the event is set, even though this defers disposal to exactly the right moment.

IMO it's a good solution. Just ignore R#.

Suggest catching an ObjectDisposedException when you call completedEvent.Set() in case the timeout has expired and the event has been disposed. I don't think this will prevent the R# warning, but it's safe.

Answer 3

I think, you must check on null, moreover you must catch this exception. Imagine what will happened if someFunc runs more than timeoutMilliseconds.