Virtual events in C #: something went wrong

    Recently, I worked on the new C # -diagnosis of V3119 for the PVS-Studio static analyzer. The purpose of diagnostics is to identify potentially unsafe constructions in the C # source code associated with the use of virtual and overridden events. Let's try to figure out what is wrong with virtual events in C #, how diagnostics work, and why Microsoft does not recommend using virtual and overridden events?


    Introduction


    I think the reader is well acquainted with the mechanisms of virtuality in C #. The simplest to understand example is the operation of virtual methods. In this case, virtuality allows one of the overrides of the virtual method to be performed in accordance with the type of runtime of the object. I will illustrate this mechanism with a simple example:

    class A
    {
      public virtual void F() { Console.WriteLine("A.F"); }
      public void G() { Console.WriteLine("A.G"); }
    }
    class B : A
    {
      public override void F() { Console.WriteLine("B.F"); }
      public new void G() { Console.WriteLine("B.G"); }
    }
    static void Main(....)
    {
      B b = new B();
      A a = b;
      a.F();
      b.F();
      a.G();
      b.G();
    }

    As a result of execution, the console will display:

    B.F
    B.F
    A.G
    B.G

    All is correct. Since both objects a and b are of the run-time type B , the call of the virtual method F () for both of these objects will lead to the call of the overridden method F () of class B. On the other hand, objects a and b differ by compilation time type , respectively, having types A and B . Therefore, method call G () for each object invokes the corresponding method for Class A or B . More about using keywordsvirtual and override can be found, for example, here .

    Similar to methods, properties, and indexers, events can also be declared virtual :

    public virtual event ....

    This can be done for both "simple" and explicitly implementing add and remove events. At the same time, working with virtual events and overridden in derived classes, it would be logical to expect from them a behavior similar to, for example, virtual methods. However, it is not. Moreover, MSDN explicitly does not recommend the use of virtual and overridden events: “Do not declare virtual events in a base class and override them in a derived class. The C # compiler does not handle these correctly and it is unpredictable whether a subscriber to the derived event will actually be subscribing to the base class event. "

    But we will not give up immediately and still try to implement "... declare virtual events in a base class and override them in a derived class".

    The experiments


    As the first experiment, we will create a console application containing the declaration and use of two virtual events in the base class (with implicit and explicit implementations of add and remove accessors ), and also containing a derived class that overrides these events:

    class Base
    {
      public virtual event Action MyEvent;
      public virtual event Action MyCustomEvent
      {
        add { _myCustomEvent += value; }
        remove { _myCustomEvent -= value; }
      }
      protected Action _myCustomEvent { get; set; }
      public void FooBase()
      {
        MyEvent?.Invoke(); 
        _myCustomEvent?.Invoke();
      }
    }
    class Child : Base
    {
      public override event Action MyEvent;
      public override event Action MyCustomEvent
      {
        add { _myCustomEvent += value; }
        remove { _myCustomEvent -= value; }
      }
      protected new Action _myCustomEvent { get; set; }
      public void FooChild()
      {
        MyEvent?.Invoke(); 
        _myCustomEvent?.Invoke();
      }
    }
    static void Main(...)
    {
      Child child = new Child();
      child.MyEvent += () =>
        Console.WriteLine("child.MyEvent handler");
      child.MyCustomEvent += () =>
        Console.WriteLine("child.MyCustomEvent handler");
      child.FooChild();
      child.FooBase();
    }

    The result of the program will be output to the console two lines:

    child.MyEvent handler
    child.MyCustomEvent handler

    Using a debugger or test output, it is easy to make sure that at the time of calling child.FooBase () the value of both variables MyEvent and _myCustomEvent is null, and the program does not “crash” only by using the conditional access operator when trying to trigger MyEvent? .Invoke ( ) and _myCustomEvent? .Invoke () .

    So, the MSDN warning was not in vain. It really doesn't work! Subscribing to virtual events of an object having the runtime type of the derived class Child does not simultaneously subscribe to events of the base class Base .

    In the case of an implicit event implementation, the compiler automatically creates add and remove accessor methods for it , as well as a delegate field, which is used to subscribe or unsubscribe. The problem, apparently, is that in the case of using a virtual event, the base and child classes will have individual (non-virtual) delegate fields associated with this event.

    In the case of an explicit implementation, this is done by the developer, who can take into account this feature of the behavior of virtual events in C #. In the above example, I did not take this feature into account by declaring the _myCustomEvent delegate property as protectedin the base and derived classes. Thus, I actually repeated the implementation provided by the compiler automatically for virtual events.

    Nevertheless, let us try to achieve the expected behavior of the virtual event using the second experiment. To do this, we use a virtual and redefined event with an explicit implementation of add and remove accessors , as well as a virtual delegate property associated with them . Let's change the program text from the first experiment a little:

    class Base
    {
      public virtual event Action MyEvent;
      public virtual event Action MyCustomEvent
      {
        add { _myCustomEvent += value; }
        remove { _myCustomEvent -= value; }
      }
      public virtual Action _myCustomEvent { get; set; }  //<= virtual
      public void FooBase()
      {
        MyEvent?.Invoke(); 
        _myCustomEvent?.Invoke();
      }
    }
    class Child : Base
    {
      public override event Action MyEvent;
      public override event Action MyCustomEvent
      {
        add { _myCustomEvent += value; }
        remove { _myCustomEvent -= value; }
      }
      public override Action _myCustomEvent { get; set; }  //<= override
      public void FooChild()
      {
        MyEvent?.Invoke(); 
        _myCustomEvent?.Invoke();
      }
    }
    static void Main(...)
    {
      Child child = new Child();
      child.MyEvent += () =>
        Console.WriteLine("child.MyEvent handler");
      child.MyCustomEvent += () =>
        Console.WriteLine("child.MyCustomEvent handler");
      child.FooChild();
      child.FooBase();
    }

    The result of the program:

    child.MyEvent handler
    child.MyCustomEvent handler
    child.MyCustomEvent handler

    Pay attention to the fact that two handler events occurred for the child.MyCustomEvent event . In debug mode, it is easy to determine that when calling _myCustomEvent? .Invoke () in the FooBase () method , the delegate value of _myCustomEvent is not null . Thus, we managed to achieve the expected behavior for virtual events only by using events with explicitly implemented add and remove accessors .

    You will say that all this, of course, is good, but we are talking about some kind of synthetic examples from the theoretical field, and let these virtual and redefined events remain there. I will give the following comments:

    • You may find yourself in a situation where you are forced to use virtual events. For example, inheriting from an abstract class in which an abstract event with an implicit implementation is declared. As a result, you will receive an overridden event in your class that you may be using. There is nothing dangerous in this until the moment you decide to inherit from your class and redefine this event again.

    • Similar designs are rare, but still found in real projects. I was convinced of this after I implemented the C # -diagnosis of V3119 for the PVS-Studio static analyzer . The diagnostic rule searches for declarations of virtual or overridden implicit implementation events that are used in the current class. An unsafe situation is when such constructions are found and the class can have heirs, and the event can be overridden (not sealed ). That is, when a situation with a redefinition of a virtual or already redefined event in a derived class is hypothetically possible. The warnings found in this way are given in the next section.

    Examples from real projects


    To test the quality of the PVS-Studio analyzer, we use a pool of test projects. After adding a new rule V3119 on virtual and redefined events to the analyzer, the entire pool of projects was checked. Let us dwell on the analysis of the warnings received.

    Roslyn


    An article was devoted to checking this project using PVS-Studio . Now, I’ll just list the analyzer warnings related to virtual and overridden events.

    PVS-Studio analyzer warning: V3119 Calling overridden event 'Started' may lead to unpredictable behavior. Consider implementing event accessors explicitly or use 'sealed' keyword. GlobalOperationNotificationServiceFactory.cs 33

    PVS-Studio analyzer warning : V3119 Calling overridden event 'Stopped' may lead to unpredictable behavior. Consider implementing event accessors explicitly or use 'sealed' keyword. GlobalOperationNotificationServiceFactory.cs 34

    private class NoOpService :
      AbstractGlobalOperationNotificationService
    {
      ....
      public override event EventHandler Started;
      public override event 
        EventHandler Stopped;
      ....
      public NoOpService()
      {
        ....
        var started = Started;  //<=
        var stopped = Stopped;  //<=
      }
      ....
    }

    In this case, we are most likely dealing with a situation of forced redefinition of virtual events. The base class AbstractGlobalOperationNotificationService is abstract and contains a definition of the Started and Stopped abstract events :

    internal abstract class 
      AbstractGlobalOperationNotificationService :
      IGlobalOperationNotificationService
    {
      public abstract event EventHandler Started;
      public abstract event 
        EventHandler Stopped;
      ....
    }

    The further use of the overridden Started and Stopped events is not entirely clear, since delegates are simply assigned to the local variables started and stopped in the NoOpService method and are not used in any way. However, this situation is potentially unsafe, as the analyzer warns.

    Sharpdevelop


    Verification of this project was also previously described in the article . Here is a list of the received warnings for the V3119 analyzer.

    PVS-Studio analyzer warning : V3119 Calling overridden event 'ParseInformationUpdated' may lead to unpredictable behavior. Consider implementing event accessors explicitly or use 'sealed' keyword. CompilableProject.cs 397

    ....
    public override event EventHandler 
      ParseInformationUpdated = delegate {};
    ....
    public override void OnParseInformationUpdated (....)
    {
      ....
      SD.MainThread.InvokeAsyncAndForget
        (delegate { ParseInformationUpdated(null, args); });  //<=
    }
    ....

    An overridden virtual event has been detected. The danger will lie in wait if we inherit from the current class and redefine the ParseInformationUpdated event in the derived class.

    PVS-Studio analyzer warning: V3119 Calling overridden event 'ShouldApplyExtensionsInvalidated' may lead to unpredictable behavior. Consider implementing event accessors explicitly or use 'sealed' keyword. DefaultExtension.cs 127

    ....
    public override event 
      EventHandler
      ShouldApplyExtensionsInvalidated;
    ....
    protected void ReapplyExtensions
      (ICollection items)
    {
      if (ShouldApplyExtensionsInvalidated != null) 
      {
        ShouldApplyExtensionsInvalidated(this,  //<=
          new DesignItemCollectionEventArgs(items));
      }
    }
    ....

    Again, the use of an overridden virtual event was detected.

    Space engineers


    And this project was previously tested using PVS-Studio. The result of the verification is given in the article . The new V3119 diagnostics generated 2 warnings.

    PVS-Studio analyzer warning: V3119 Calling virtual event 'OnAfterComponentAdd' may lead to unpredictable behavior. Consider implementing event accessors explicitly. MyInventoryAggregate.cs 209

    PVS-Studio analyzer warning: V3119 Calling virtual event 'OnBeforeComponentRemove' may lead to unpredictable behavior. Consider implementing event accessors explicitly. MyInventoryAggregate.cs 218

    ....
    public virtual event 
      Action
      OnAfterComponentAdd;
    public virtual event 
      Action
      OnBeforeComponentRemove;
    ....
    public void AfterComponentAdd(....)
    {
      ....
      if (OnAfterComponentAdd != null)
      {
        OnAfterComponentAdd(....);  // <=
      }                
    }
    ....
    public void BeforeComponentRemove(....)
    {
      ....
      if (OnBeforeComponentRemove != null)
      {
        OnBeforeComponentRemove(....);
      }
    }
    ....

    Here we are dealing with the declaration and use of virtual events rather than redefined ones. In general, the situation does not differ from previously considered.

    Ravendb


    The RavenDB project is a so-called “NoSQL” (or document-oriented) database. Its detailed description is available on the official website . The project was developed using .NET and its source code is available on GitHub . Check of RavenDB by the PVS-Studio analyzer revealed three warnings of V3119.

    PVS-Studio analyzer warning: V3119 Calling overridden event 'AfterDispose' may lead to unpredictable behavior. Consider implementing event accessors explicitly or use 'sealed' keyword. DocumentStore.cs 273

    PVS-Studio Analyzer Warning: V3119Calling overridden event 'AfterDispose' may lead to unpredictable behavior. Consider implementing event accessors explicitly or use 'sealed' keyword. ShardedDocumentStore.cs 104

    Both of these warnings are issued for similar code snippets. Consider one of these fragments:

    public class DocumentStore : DocumentStoreBase
    {
      ....
      public override event EventHandler AfterDispose;
      ....
      public override void Dispose()
      {
        ....
        var afterDispose = AfterDispose;  //<=
        if (afterDispose != null)
          afterDispose(this, EventArgs.Empty);
      }
      ....
    }

    The OverDispose event overridden in the DocumentStore class is declared as abstract in the base abstract class DocumentStoreBase :

    public abstract class DocumentStoreBase : IDocumentStore
    {
      ....
      public abstract event EventHandler AfterDispose;
      ....
    }

    As in the previous examples, the analyzer warns us of the potential danger if the AfterDispose virtual event is overridden and used in classes derived from DocumentStore .

    PVS-Studio analyzer warning: V3119 Calling virtual event 'Error' may lead to unpredictable behavior. Consider implementing event accessors explicitly. JsonSerializer.cs 1007
    ....
    public virtual event EventHandler Error;
    ....
    internal void OnError(....)
    {
      EventHandler error = Error; //<=
      if (error != null)
        error(....);
    }
    ....

    This is where the announcement and use of the virtual event takes place. Once again, there is a risk of uncertain behavior.

    Conclusion


    I think this is where we can finish our study and conclude that it is really not worth using implicitly implemented virtual events. Due to the nature of their implementation in C #, the use of such events can lead to undefined behavior. In case you are still forced to use redefined virtual events (for example, when inheriting from an abstract class), this should be done with caution, using explicitly defined add and remove accessors . You can also use the sealed keyword when declaring a class or event. And, of course, you should use static code analysis tools, for example, PVS-Studio .


    If you want to share this article with an English-speaking audience, then please use the link to the translation: Sergey Khrenov. Virtual events in C #: something went wrong .

    Have you read the article and have a question?
    Often our articles are asked the same questions. We collected the answers here: Answers to questions from readers of articles about PVS-Studio, version 2015 . Please see the list.

    Also popular now: