Code improvement: Better alternatives to this pattern?

In a similar question:

What is this pattern called? Soft Lock?

I was asking about the name of th开发者_开发百科e pattern for the code listing below.

  public class MyClass
  {
    public event EventHandler MyEvent;
    private bool IsHandlingEvent = false;

    public MyClass()
    {
      MyEvent += new EventHandler(MyClass_MyEvent);
    }

    void MyClass_MyEvent(object sender, EventArgs e)
    {
      if (IsHandlingEvent) { return; }

      IsHandlingEvent = true;
      {
        // Code goes here that handles the event, possibly invoking 'MyEvent' again.
        // IsHandlingEvent flag is used to avoid redundant processing.  What is this
        // technique, or pattern called.
        // ...
      }
      IsHandlingEvent = false;
    }
  }

It seems that most of the conversation was centered around why we should an should not do this, so I think that this question provides a better forum to tackle the problem and address all of the issues. What is the better / proper way to handle this?

There are series of problems with that pattern. If you want to invoke the handler only once, you would do something like this:

 protected static object _lockObj = new object();
 protected static bool _isHandled = false;    

 void MyClass_MyEvent(object sender, EventArgs e)
 {
     if(_isHandled)
       return;

     lock(_lockObj)
     {
         if(_isHandled)
            return;

         _isHandled = true;

         MyOtherPossiblyRecursiveMethod(); // Actually does all your work

         _isHandled = false;
     }
 }

 void MyOtherPossiblyRecursiveMethod()
 {
 }

This way, only one thread should be able to access the actual work method.

I will use something like:

using( var sl = new SoftLock() )
{
   sl.Execute(()=>{....});
}

the execute will raise the internal boolean to prevent re-entering. In the dispose that flag would be resetted. Execute will call the lambda just if the flag is false. This is to ensure flag go to false even if exception happens ( causing handler never executed ) and maybe is a little better to see. Of course this is not thread safe, as the original code, but this because we are talking about preventing double execution from the same thread.

The original code is a sufficient (and very lightweight) way to prevent recursion in a single-threaded app. So if during your event handling function you could get into code that might be firing the event again you will not enter infinite recursion.

But the code is not sufficient to prevent access from multiple threads, due to the potential for race conditions. If you need to ensure only one thread gets to run this event, then you should use a stronger locking mechanism, like a Mutex or Semaphore.

The following works in single- and multi-threaded scenarios and is exception-safe... also if need be it can be modified to allow for a certain level of reentrancy (for example 3 levels)...

public class MyClass
{
public event EventHandler MyEvent;
private int IsHandlingEvent = 0;

public MyClass()
{
  MyEvent += new EventHandler(MyClass_MyEvent);
}

void MyClass_MyEvent(object sender, EventArgs e)
{
 // this allows for nesting if needed by comparing for example < 3 or similar
 if (Interlocked.Increment (ref IsHandlingEvent) == 1 )
 {
  try {
      }
  finally {};  
 } 

 Interlocked.Decrement (ref IsHandlingEvent);
}
}