Making member virtual prevents calling default interface implementation and causes StackOverflowException in C# 8

Question

Consider the code:

class ChildClass : BaseClass {

    public void Method1() {} //some other method

}

abstract class BaseClass : IChildInterface {

    public
    virtual //<- If we add virtual so that this method can be overridden by ChildClass, we get StackOverflowException and DoWork() implementation in IChildInterface is never called.
    void DoWork() {
     //base class specific implmentation
     ((IChildInterface)this).DoWork(); //call into default implementation provided by IChildInterface
    }

}

interface IChildInterface : IBaseInterface {

    void IBaseInterface.DoWork() {
     //implmentation
    }

}

interface IBaseInterface {

    void DoWork();

}

The problem is that if we mark DoWork() in BaseClass as virtual so that it can be overridden by child classes, it prevents it from calling into IChildInterface's default implementation of DoWork(), causing StackOverflowException.

If we remove virtual modifier from DoWork() in the BaseClass, everything works and the IChildInterface's default implementation of DoWork() is called.

Is such a behavior a bug, or by design?

Is there a way to make it possible for some child classes provide their own implementation of DoWork() (thus overriding BaseClass's implementation) but still being able to use IChildInterface's default implementation of DoWork()?

Oared

It looks like you have over-engineered your code, do you really need so many interfaces ? – Logos

@Logos Yes! This is in fact a simplified example. But that's what you get in complex apps with multiple libraries and separation of concerns involved. – Oared

can you refactor your code to make it at least compilable? Easier to look into this. your IChildInterface has a method implementation. Also your base class implements IChildInterface, thats probably supposed to be IBaseInterface. – Missie

interfaces do not have implementations you need to have something which can implement a interface. remove the void IBaseInterface.DoWork() { from IChildInterface and add it to ChildClass also you could of made it an abstract method. – Rickert

@MichaelSander: Yes, IChildInterface has a method implementation because that's the default implementation, as stated in the question. Default interface implementations are part of C# 8. – Whitefly

@Rickert Interfaces can have implementations in C# 8. – Whitefly

The example can be simplified to a single interface and a single implementation - gist.github.com/jskeet/5dfa291d2ae8b798a09f1c81f70ae2af. But the problem remains when you remove the virtual modifier anyway. – Whitefly

@JonSkeet Ooops, didn't work with that so far. And not sure if I like this feature :) – Missie

@FitDev given how fresh DIMs are, you won't get such situations in any library yet. Interfaces were never used that way, and can't be used now either. They aren't base classes so there's no base implementation available – Camilacamile

Fundamentally, this does sound like something that should be designed differently, by using two different methods. Even if you can find some way to get this to work, it'll still be hugely confusing. – Whitefly

Answer 1

You're calling BaseClass.DoWork recursively which, if you're lucky, will result in a StackOverflowException. If the call was the last one in the method, you'd get an infinite recursion due to tail call optimizations. You'd end up with a core stuck at 100% until you killed the app.

This code :

public virtual void DoWork() {
   ((IChildInterface)this).DoWork(); by IChildInterface
}

Is identical to :

//That's the actual implementation of the interface method
public virtual void DoWork() {
     DoWork(); 
}

The virtual keyword doesn't matter. You'd still get infinite recursion without it. Whether it exists or not, this line throws a StackOverflowException after a while :

new ChildClass().DoWork();

When you implemented BaseClass.DoWork that became the single implementation available to everyone, unless overridden by a child class.

Interfaces are not abstract classes, even in C# 8. A default method implementation is not an actual method. As the name says, it's a default implementation. It's used when there's no better implementation available. You can't call the default implementation when the method is already implemented in a class.

In fact, in almost every case you wouldn't expect the default method to be called. DIMs are called explicitly through the interface, the same way explicit interface implementations are used. Callers of the method expect the most-derived implementation to run, not the base or mid-level one.

Besides, even on previous C# versions you wouldn't expect casting to an interface to change which method is actually called. You'd expect that only with classes. To call a base class implementation you'd use the base keyword. The base class of BaseClass though is Object which doesn't have a DoWork method.

If you used :

void DoWork() {
    base.DoWork(); 
}

You'd get a CS0117: 'object' does not contain a definition for 'DoWork'

Update

The C# design team has already though about this. This couldn't be implemented efficiently without runtime support and was cut i May 2019. Runtime optimizations is what makes DIM calls as cheap as other calls, without boxing etc.

The proposed syntax is a base(IMyInterface) call :

interface I1
{ 
    void M(int) { }
}

interface I2
{
    void M(short) { }
}

interface I3
{
    override void I1.M(int) { }
}

interface I4 : I3
{
    void M2()
    {
        base(I3).M(0) // What does this do?
    }
}

Camilacamile

Thank you for your detailed response. However it does not seem to be working the same way on my end. In particular, you say "You can't call the default implementation when the method is already implemented in a class." But I can and I did! All I have to do is remove the virtual keyword from the DoWork() in the BaseClass and both the BaseClass's part and the default implementation in the IChildInterface of DoWork() will be executed just fine and only once and in correct order. – Oared

@FitDev the code you posted doesn't do that at all. It goes into an infinite recursion, virtual or not. Which .NET Core runtime version are you using? – Camilacamile

Also, my understanding from what I have seen and read regarding DIM features of c# 8 is that you actually do use the syntax ((IInterface)this).DefaultImplementationDefinedInIInterface() to explicitly execute the code that's defined in the DIM. – Oared

That's very strange. Because in my case (Net Core 3.1 actually) it works fine without the virtual keyword. – Oared

@FitDev no in both. No repro in 3.1 on the code, and no, you don't use that syntax to call the DIM. You use that to call the implementation of the method, whether it's from the interface or an implementation in the class. It's the syntax used for explicit interface implementations, not specific to DIMs – Camilacamile

@FitDev the code you posted doesn't actually execute anything. Post an actual, runnable example that demonstrates the problem. A console app and a Main method that runs the code you posted – Camilacamile

You are right. My actual code was a bit more involved, hence I simplified it here. And what ended up happening was that the BaseClass's DoWork() was never actually called, and only IChildInterface.DoWork() was ever actually called - hence there was no problem. Adding virtual to BaseClass's DoWork() in my case caused the calls to go through BaseClass resulting in StackOverflow. Thus you are correct that there is no difference in the end. – Oared

Answer 2

As all methods inside interfaces are virtual by default the DoWork is virtual inside every each of these definitions/implementations you provided except the ChildClass. When you explicitly use DoWork of IChildInterface it uses BaseClass.DoWork implicitly which then uses ((IChildInterface)this).DoWork(); explicitly again. And so on. You have this loop that is never ending, hence you're getting the StackOverflow.

Defective

"As all methods inside interfaces are virtual by default"????? What language you are talking about? – Communicative

Though technically true that the methods in the interface are flagged as virtual in the metadata, the classes that implement them doesn't inherit this trait automatically, well not exactly. The implementing method is marked as virtual and sealed so that it cannot be overridden, unless the class that implements the method explicitly declares the method as virtual. – Contuse

The problem here, however, is like Panagiotis said in his answer, there is really just 1 method in play here by the time you implement the method in the class. The default interface methods are only present until you explicitly implement it, then they sort of cease to exist for that type. So saying that you're not really calling the interface method is wrong, that method is no longer there. – Contuse

@AlexeiLevenkov I wouldn't argue with you. There's Jeffrey Richter who clearly pointed this out for me. I didn't write all the possible cases. But in that particular case it does become virtual as DoWork of BaseClass marked as virtual. – Defective

@LasseV.Karlsen I mean there is an attemt to call IChildInterface.DoWork but as it merked as virtual in that particular case the BaseClass.DoWork is invoked. – Defective

That may also be true but there is no other method available to call anyway. – Contuse

Answer 3

For the sake of future readers...

While the accepted answer provided by @Panagiotis is correct, in that there is no difference whether virtual modifier is present or not and StackOverflowExcpetion will occur in any case, I wanted to provide a concrete answer to the question that I settled on.

The whole point of implementing DoWork() in the IChildInterface as opposed to in a class was for code reuse and staying "DRY". Classes that implement IChildInterface should however be able to add their own functionality on top of what's provided by IChildInterface.

And therein lies a problem, as calling ((IChildInterface)this).DoWork(); from any class (abstract or not) that implements IChildInterface will result in infinite recursion. The only reasonable way out seems to use protected static members (as in fact is suggested in the Microsoft Docs):

class ChildClass : BaseClass {

    public void Method1() {} //some other method

}

abstract class BaseClass : IChildInterface {

    public virtual void DoWork() {
     // Base class specific implementation here

     // Then call into default implementation provided by IChildInterface:
     // Instead of this: ((IChildInterface)this).DoWork();
     // Use static implementation:
     IChildInterface.DoWork(this);
    }

}

interface IChildInterface : IBaseInterface {

    protected static void DoWork(IChildInterface it){
      // Implementation that works on instance 'it'
    }
    void IBaseInterface.DoWork() => IChildInterface.DoWork(this);

}

interface IBaseInterface {

    void DoWork();

}

In the above solution we are staying "DRY" by still having a single (core) implementation of DoWork(), but it is located in a protected static member of the interface IChildInterface instead of being part of its inheritance hierarchy.

Then, as far as the inheritance hierarchy is concerned, all interfaces / classes deriving from / implementing IChildInterface could simply use IChildInterface.DoWork(this) to access the default implementation. This applies to the IChildInterface itself.

Oared