﻿# Virtual events in C\#: something went wrong

Not so long ago I was working on a new C\# diagnostic \- V3119 \- for the PVS\-Studio static code analyzer\. The function of this diagnostic is to detect potentially unsafe constructions in the source code of C\#, related to the usage of virtual and overridden events\. Let's try to sort out, what's wrong with virtual events in C\# \- the principle of this diagnostic, and why Microsoft doesn't recommend using virtual and overridden events\. 

![0453_VirtualEvents/image1.png](https://import.viva64.com/docx/blog/0453_VirtualEvents/image1.png)

## Introduction

I think our readers are quite aware of what virtual mechanisms in C\# are\. The simplest example would be an example of virtual methods\. In this case, virtuality allows to run the overridden virtual method according to the object's run\-time type\. I'll give an illustration using a simple example\. 

```cpp
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 we will have the following: 

```cpp
B.F
B.F
A.G
B.G
```

Everything is correct\. Since both objects a and b have the **B** **type** **at** **run\-time,** then the call of the virtual method _F\(\)_ for both these objects will lead to the call of the overridden method _F\(\)_ of _B_ class\. On the other hand, _a_ and _b_ objects differ in the **compile** **time** **type**, having _A_ and _B_ types accordingly\. That's why the call of the _G\(\)_ method for each of these objects leads to the call of the corresponding method for _A_ or _B_ class\. You can find more details about the usage of the keywords virtual and override [here](https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/keywords/virtual)\. 

Like methods, properties and indicators, [events](https://docs.microsoft.com/en-us/dotnet/csharp/programming-guide/events/) can also be declared as virtual: 

```cpp
public virtual event ....
```

You can do this as for "simple" and for events, explicitly implementing accessors _add_ and _remove_\. So, working with virtual and overridden events in the derived classes, it would be logical to expect behavior similar to the behavior of the virtual methods\. But this is not the case\. Moreover, [MSDN](https://docs.microsoft.com/en-us/dotnet/csharp/programming-guide/events/how-to-raise-base-class-events-in-derived-classes) directly say that they **do not recommend** using 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"\.

However, we do not give up, so let us try to implement "\.\.\. declare virtual events in a base class and override them in a derived class"\.

## Experiments

As the first experiment, let us create a console application, where we will have two virtual events in the base class declared and used \(with explicit and implicit implementation of add and remove accessors\) and a derived class, overriding these events:

```cpp
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 exectution will be: 

```cpp
child.MyEvent handler
child.MyCustomEvent handler
```

Using the debugger or a test output, it's easy to make sure that at the time of the _child\.FooBase\(\) _call, the values of both variables _MyEvent _and _\_myCustomEvent _are null, and the program doesn't crash only because of the conditional access operator upon the attempt to initialize the events _MyEvent?\.Invoke\(\)_ and _\_myCustomEvent?\.Invoke\(\)_\.

So, the MSDN warning was not in vain\. It really doesn't work\. The subscription to the virtual events of an object using the Child run time type, doesn't lead to a simultaneous subscription to the events of the Base class\. In the case of implicit implementation of the event, the compiler automatically creates methods\-accessors for it  \- _add_ and _remove_, and also a delegate field, which is used to subscribe and unsubscribe\. The problem, apparently, is that if you use a virtual event, the basic and child classes will have individual \(not virtual\) delegate\-fields that are connected with this event\. 

In the case of explicit implementation \- it is a developer who does that, and takes into account this peculiarity of virtual events behavior in C\#\. In the example above, I didn't take into account this peculiarity, declaring the delegate property _\_myCustomEvent_ as _protected_ in the base and derived classes\. Thus, I actually repeated the implementation provided automatically by the compiler for virtual events\.

Let's try to achieve the expected behavior of a virtual event, with the help of the second experiment\. To do this, let's use a virtual and overridden event with explicit implementation of _add_ and _remove_ accessors, and also a **virtual** delegate property, related to it\. Let's change the text of the program from the first experiment:

```cpp
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();
}
```

Result of the program execution: 

```cpp
child.MyEvent handler
child.MyCustomEvent handler
child.MyCustomEvent handler
```

Take note of the fact that there were two executions of the handler for the event _child\.MyCustomEvent\. _In debugging mode, it is easy to detect that now, upon the call of _\_myCustomEvent?\.Invoke\(\) _in the _FooBase\(\) _method, the value of the delegate is not null\. Thus, we managed to get the expected behavior for virtual events only by using events with explicitly implemented accessors _add_ and _remove_\. 

You may say that that's great, of course, but we are talking about some synthetic examples from the theoretical field, so let these virtual and overridden events remain there\. I'll give the following comments: 

* You may find yourself in a situation where you're forced to use virtual events\. For example, inheriting from an abstract class that has an abstract event, declared with an implicit implementation\. As a result, you get in your class, an overridden event, which you may use later\. There is nothing dangerous until you choose to inherit from your class, and override this event again\.
* Such constructions are quite rare, but still they can be found in real projects\. I was convinced of this after I implemented the C\# diagnostic [V3119](https://pvs-studio.com/en/docs/warnings/v3119/) for the static code analyzer [PVS\-Studio](https://pvs-studio.com/en/pvs-studio/)\. The diagnostic looks for declarations of virtual or overridden events with implicit implementation that are used in the current class\. A situation is considered unsafe when such constructions are found, and the class can have derived classes, and the event can be overridden \(not _sealed_\)\. That is, when hypothetically it is possible to have a situation with the overriding of a virtual or an already overridden event in a derived class\. Warnings that were found in such a way are given in the next section\. 

## Examples from real projects

To test the quality of PVS\-Studio analyzer's work, we use a pool of test projects\. After adding the new rule, V3119, to the analyzer that is devoted to virtual and overridden events, we did a check of the whole pool of projects\. Let's see what warnings we got\. 

### Roslyn

This project has been previously checked, and you can find the article [here](https://pvs-studio.com/en/blog/posts/csharp/0363/)\. Now I just give a list of analyzer warnings that are related to virtual and overridden virtual events\.

**PVS\-Studio warning**: [V3119](https://pvs-studio.com/en/docs/warnings/v3119/) Calling overridden event 'Started' may lead to unpredictable behavior\. Consider implementing event accessors explicitly or use 'sealed' keyword\. GlobalOperationNotificationServiceFactory\.cs 33

**PVS\-Studio warning**: [V3119](https://pvs-studio.com/en/docs/warnings/v3119/) Calling overridden event 'Stopped' may lead to unpredictable behavior\. Consider implementing event accessors explicitly or use 'sealed' keyword\. GlobalOperationNotificationServiceFactory\.cs 34

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

In this case, we are most likely dealing with a situation of forced overriding of virtual events\. The base class _AbstractGlobalOperationNotificationService_ is abstract, and has declaration of abstract events _Started_ and _Stopped_:

```cpp
internal abstract class 
  AbstractGlobalOperationNotificationService :
  IGlobalOperationNotificationService
{
  public abstract event EventHandler Started;
  public abstract event 
    EventHandler<GlobalOperationEventArgs> Stopped;
  ....
}
```

It's not quite clear how the overridden events _Started_ and _Stopped_ will be used further on, because the delegates are just assigned to the local variables _started_ and _stopped_, and aren't used in the _NoOpService_ in any way\. However, this situation is potentially unsafe, and the analyzer warns about this\.

### SharpDevelop

The analysis of the project has also been previously described in the [article](https://pvs-studio.com/en/blog/posts/csharp/0359/)\. I'll give here a list of the V3119 analyzer warnings\. 

**PVS\-Studio warning**: [V3119](https://pvs-studio.com/en/docs/warnings/v3119/) Calling overridden event 'ParseInformationUpdated' may lead to unpredictable behavior\. Consider implementing event accessors explicitly or use 'sealed' keyword\. CompilableProject\.cs 397

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

The analyzer detected usage of an overridden virtual event\. We'll have a dangerous situation in case of inheritance from the current class, and overriding of the _ParseInformationUpdated_ event in the derived class\. 

**PVS\-Studio warning**: [V3119](https://pvs-studio.com/en/docs/warnings/v3119/) Calling overridden event 'ShouldApplyExtensionsInvalidated' may lead to unpredictable behavior\. Consider implementing event accessors explicitly or use 'sealed' keyword\. DefaultExtension\.cs 127

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

Again, the analyzer detected usage of an overridden virtual event\. 

### Space Engineers

This project was also previously checked by PVS\-Studio\. You can find the results of the analysis in [this article](https://pvs-studio.com/en/blog/posts/csharp/0376/)\. The new V3119 diagnostics issued 2 warnings\.

**PVS\-Studio warning**: [V3119](https://pvs-studio.com/en/docs/warnings/v3119/) Calling virtual event 'OnAfterComponentAdd' may lead to unpredictable behavior\. Consider implementing event accessors explicitly\. MyInventoryAggregate\.cs 209

**PVS\-Studio warning**: [V3119](https://pvs-studio.com/en/docs/warnings/v3119/) Calling virtual event 'OnBeforeComponentRemove' may lead to unpredictable behavior\. Consider implementing event accessors explicitly\. MyInventoryAggregate\.cs 218

```cpp
....
public virtual event 
  Action<MyInventoryAggregate, MyInventoryBase>
  OnAfterComponentAdd;
public virtual event 
  Action<MyInventoryAggregate, MyInventoryBase>
  OnBeforeComponentRemove;
....
public void AfterComponentAdd(....)
{
  ....
  if (OnAfterComponentAdd != null)
  {
    OnAfterComponentAdd(....);  // <=
  }                
}
....
public void BeforeComponentRemove(....)
{
  ....
  if (OnBeforeComponentRemove != null)
  {
    OnBeforeComponentRemove(....);
  }
}
....
```

We are dealing here with the declaration and usage not of overridden, but of virtual events\. In general, the situation is no different from the previous ones\.

### RavenDB

The RavenDB project is a so called "NoSQL" \(or document\-oriented\) database\. Its detailed description is available on the [official website](https://ravendb.net/)\. The project is developed using \.NET, and the source code is available on [GitHub](https://github.com/ravendb/ravendb)\. The analysis of RavenDB by the PVS\-Studio analyzer detected three V3119 warnings\. 

**PVS\-Studio warning**: [V3119](https://pvs-studio.com/en/docs/warnings/v3119/) Calling overridden event 'AfterDispose' may lead to unpredictable behavior\. Consider implementing event accessors explicitly or use 'sealed' keyword\. DocumentStore\.cs 273

**PVS\-Studio warning**: [V3119](https://pvs-studio.com/en/docs/warnings/v3119/) Calling overridden event 'AfterDispose' may lead to unpredictable behavior\. Consider implementing event accessors explicitly or use 'sealed' keyword\. ShardedDocumentStore\.cs 104

Both of these warnings were issued for similar code fragments\. Let's take a look at one such fragment: 

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

The event _AfterDispose_, overridden in the class _DocumentStore_, is declared as abstract in the base abstract class _DocumentStoreBase_:

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

As in the previous examples, the analyzer warns us of the potential danger, should the virtual event _AfterDispose_ be overridden and be used in the classes derived from _DocumentStore_\.

**PVS\-Studio warning:** [V3119](https://pvs-studio.com/en/docs/warnings/v3119/) Calling virtual event 'Error' may lead to unpredictable behavior\. Consider implementing event accessors explicitly\. JsonSerializer\.cs 1007

```cpp
....
public virtual event EventHandler<ErrorEventArgs> Error;
....
internal void OnError(....)
{
  EventHandler<ErrorEventArgs> error = Error; // <=
  if (error != null)
    error(....);
}
....
```

Here we have declaration and use of a virtual event\. Again, there is a risk of undefined behavior\.

## Conclusion

I think we can stop here and draw the conclusion that we really shouldn't use implicitly implemented virtual events\. Due to the specifics of their implementation in C\#, the usage of such events can lead to undefined behavior\. In case you have to use overridden virtual events \(for example, upon the derivation from an abstract class\), this should be done with caution, using explicitly defined accessors _add_ and _remove\._ You can also use the keyword sealed, when declaring a class or an event\. And of course, you should use static code analysis tools, like [PVS\-Studio](https://pvs-studio.com/en/pvs-studio/) for example\.