Putting a base in the middle
Here’s a crazy-seeming but honest-to-goodness real customer scenario that got reported to me recently. There are three DLLs involved, Alpha.DLL, Bravo.DLL and Charlie.DLL. The classes in each are:
public class Alpha // In Alpha.DLL
{
public virtual void M()
{
Console.WriteLine("Alpha");
}
}
public class Bravo: Alpha // In Bravo.DLL
{
}
public class Charlie : Bravo // In Charlie.DLL
{
public override void M()
{
Console.WriteLine("Charlie");
base.M();
}
}
Perfectly sensible. You call M on an instance of Charlie and it says “Charlie / Alpha”.
Now the vendor who supplies Bravo.DLL ships a new version which has this code:
public class Bravo: Alpha
{
public override void M()
{
Console.WriteLine("Bravo");
base.M();
}
}
The question is: what happens if you call Charlie.M without recompiling Charlie.DLL, but you are loading the new version of Bravo.DLL?
The customer was quite surprised that the output is still “Charlie / Alpha”, not “Charlie / Bravo / Alpha”.
This is a new twist on the brittle base class failure; at least, it’s new to me.
Customer: What’s going on here?
When the compiler generates code for the base call, it looks at all the metadata and sees that the nearest valid method that the base call can be referring to is Alpha.Foo. So we generate code that says “make a non-virtual call to Alpha.Foo”. That code is baked into Charlie.DLL and it has the same semantics no matter what Bravo.DLL says. It calls Alpha.Foo.
Customer: You know, if you generated code that said “make a non-virtual call to Bravo.Foo”, the CLR will fall back to calling Alpha.Foo if there is no implementation of Bravo.Foo.
No, I didn’t know that actually. I’m slightly surprised that this doesn’t produce a verification error, but, whatever. Seems like a plausible behaviour, albeit perhaps somewhat risky. A quick look at the documented semantics of the call instruction indicates that this is the by-design behaviour, so it would be legal to do so.
Customer: Why doesn’t the compiler generate the call as a call to Bravo.Foo? Then you get the right semantics in my scenario!
Essentially what is happening here is the compiler is generating code on the basis of today's static analysis, not on the basis of what the world might look like at runtime in an unknown future. When we generate the code for the base call we assume that there are not going to be changes in the base class hierarchy after compilation. That seemed at the time to be a reasonable assumption, though I can see that in your scenario, arguably it is not.
As it turns out, there are two reasons to do it the current way. The first is philosophical and apparently unconvincing. The second is practical.
Customer: What’s the philosophical justification?
There are two competing "mental models" of what "base.Foo" means.
The mental model that matches what the compiler currently implements is “a base call is a non-virtual call to the nearest method on any base class, based entirely on information known at compile time.”
Note that this matches exactly what we mean by "non-virtual call". An early-bound call to a non-virtual method is always a call to a particular method identified at compile time. By contrast, a virtual method call is based at least in part on runtime analysis of the type hierarchy. More specifically, a virtual method identifies a "slot" at compile time but not the "contents" of that slot. The "contents" – the actually method to call – is identified at runtime based on what the runtime type of the receiver stuffed into the virtual method slot.
Your mental model is “a base call is a virtual call to the nearest method on any base class, based on both information known at runtime about the actual class hierarchy of the receiver, and information known at compile time about the compile-time type of the receiver.”
In your model the call is not actually virtual, because it is not based upon the contents of a virtual slot of the receiver. But neither is it entirely based on the compile-time knowledge of the type of the receiver! It's based on a combination of the two. Basically, it’s what would have been the non-virtual call in the counterfactual world where the compiler had been given correct information about what the types actually would look like at runtime.
A developer who has the former mental model (like, say, me) would be deeply surprised by your proposed behavior. If the developer has classes Giraffe, Mammal and Animal, Giraffe overrides virtual method Animal.Feed, and the developer says base.Feed in Giraffe, then the developer is thinking either like me:
I specifically wish Animal.Feed to be called here; if at runtime it turns out that evil hackers have inserted a method Mammal.Feed that I did not know about at compile time, I still want Animal.Feed to be called. I have compiled against Animal.Feed, I have tested against that scenario, and that call is precisely what I expect to happen. A base call gives me 100% of the safe, predictable, understandable, non-dynamic, testable behavior of any other non-virtual call. I rely upon those invariants to keep my customer's data secure.
Basically, this position is "I trust only what I can see when I wrote the code; any other code might not do what I want safely or correctly".
Or like you:
I need the base class to do some work for me. I want something on some base class to be called. Animal.Feed or Mammal.Feed, I don't care, just pick the best one - whichever one happens to be "most derived" in some future version of the world - by doing that analysis at runtime. In exchange for the flexibility of being able to hot-swap in new behavior by changing the implementation of my base classes without recompiling my derived classes, I am willing to give up safety, predictability, and the knowledge that what runs on my customer's machines is what I tested.
Basically, this position is "I trust that the current version of my class knows how to interpret my request and will do so safely and correctly, even if I've never once tested that."
Though I understand your point of view, I’m personally inclined to do things the safe, boring and sane way rather than the flexible, dangerous and interesting way. However, based on the several dozen comments on the first version of this article, and my brief poll of other members of the C# compiler team, I am in a small minority that believes that the first mental model is the more sensible one.
Customer: The philosophical reason is unconvincing; I see a base call as meaning “call the nearest thing in the virtual hierarchy”. What’s the practical concern?
In the autumn of 2000, during the development of C# 1.0, the behaviour of the compiler was as you expect: we would generate a call to Bravo.M and allow the runtime to resolve that as either a call to Bravo.M if there is one or to Alpha.M if there is not. My predecessor Peter Hallam then discovered the following case. Suppose the new hot-swapped Bravo.DLL is now:
public class Bravo: Alpha
{
new private void M()
{
Console.WriteLine("Bravo");
}
}
Now what happens? Bravo has added a private method, and one of our design principles is that private methods are invisible implementation details; they do not have any effect on the surrounding code that cannot see them. If you hot-swap in this code and the call in Charlie is realized as a call to Bravo.M then this crashes the runtime. The base call resolves as a call to a private method from outside the method, which is not legal. Non-virtual calls do matching by signature, not by virtual slot.
The CLR architects and the C# architects considered many possible solutions to this problem, including adding a new instruction that would match by slot, changing the semantics of the call instruction, changing the meaning of "private", implementing name mangling in the compiler, and so on. The decision they arrived at was that all of the above were insanely dangerous considering how late in the ship cycle it was, how unlikely the scenario is, and the fact that this would be enabling a scenario which is directly contrary to good sense; if you change a base class then you should recompile your derived classes. We don't want to be in the business of making it easier to do something dangerous and wrong.
So they punted on the issue. The C# 1.0 compiler apparently did it the way you like, and generated code that sometimes crashed the runtime if you introduced a new private method: the original compilation of Charlie calls Bravo.M, even if there is no such method. If later there turns out to be an inaccessible one, it crashes. If you recompile Charlie.DLL, then the compiler notices that there is an intervening private method which will crash the runtime, and generates a call to Alpha.M.
This is far from ideal. The compiler is designed so that for performance reasons it does not load the potentially hundreds of millions of bytes of metadata about private members from referenced assemblies; now we have to load at least some of that. Also, this makes it difficult to use tools such as ASMMETA which produce "fake" versions of assemblies which are then later replaced with real assemblies. And of course there is always still the crashing scenario to worry about.
The situation continued thusly until 2003, at which point again the C# team brought this up with the CLR team to see if we could get a new instruction defined, a "basecall" instruction which would provide an exact virtual slot reference, rather than doing a by-signature match as the non-virtual call instruction does now. After much debate it was again determined that this obscure and dangerous scenario did not meet the bar for making an extremely expensive and potentially breaking change to the CLR.
Concerned over all the ways that this behaviour was currently causing breaks and poor performance, in 2003 the C# design team decided to go with the present approach of binding directly to the slot as known at compile time. The team all agreed that the desirable behaviour was to always dynamically bind to the closest base class -- a point which I personally disagree with, but I see their point. But given the costs of doing so safely, and the fact that hot-swapping in new code in the middle of a class hierarchy is not exactly a desirable scenario to support, it's better to sometimes force a recompilation (that you should have done anyways) than to sometimes crash and die horribly.
Customer: Wow. So, this will never change, right?
Wow indeed. I learned an awful lot today. One of these days I need to sit down and just read all five hundred pages of the C# 1.0 and 2.0 design notes.
I wouldn’t expect this to ever change. If you change a base class, recompile your derived classes. That’s the safe thing to do. Do not rely on the runtime fixing stuff up for you when you hot-swap in a new class in the middle of a class hierarchy.
UPDATE: Based on the number of rather histrionic comments I've gotten over the last 24 hours, I think my advice above has been taken rather out of the surrounding context. I'm not saying that every time someone ships a service pack that has a few bug fixes that you are required to recompile all your applications and ship them again. I thought it was clear from the context that what I was saying was that if you depend upon base type which has been updated then:
(1) at the very least test your derived types with the new base type -- your derived types are relying on the mechanisms of the base types; when a mechanism changes, you have to re-test the code which relies upon that mechanism.
(2) if there was a breaking change, recompile, re-test and re-ship the derived type. And
(3) you might be surprised by what is a breaking change; adding a new override can potentially be a breaking change in some rare cases.
I agree that it is unfortunate that adding a new override is in some rare cases a semantically breaking change. I hope you agree that it is also unfortunate that adding a new private method was in some rare cases a crash-the-runtime change in C# 1.0. Which of those evils is the lesser is of course a matter of debate; we had that debate between 2000 and 2003 and I don't think its wise or productive to second-guess the outcome of that debate now.
The simple fact of the matter is that the brittle base class problem is an inherant problem with the object-oriented-programming pattern. We have worked very hard to design a language which minimizes the likelihood of the brittle base class problem biting people. And the base class library team works very hard to ensure that service pack upgrades introduce as few breaking changes as possible while meeting our other servicing goals, like fixing existing problems. But our hard work only goes so far, and there are more base classes in the world that those in the BCL.
If you find that you are getting bitten by the brittle base class problem a lot, then maybe object oriented programming is not actually the right solution for your problem space; there are other approaches to code reuse and organization which are perfectly valid that do not suffer from the brittle base class problem.
Comments
Anonymous
March 29, 2010
(Before reading this article, I actually thought the base call was a "magic virtual base call" that called the nearest base class function.) But..., the whole point of the "runtime-assembly-binding bindingRedirect" in configuration files is to be able to replace assemblies with new versions without recompiling the code. It's extremly important that one doesn't place code in new overrides. When Microsoft releases service packs for the framework, do they (you) never create new overrides in non-sealed public classes? I thought this was happening frequently especially in UI-control-libraries where you are supposed to inherit from base classes. If they do, everyone has to re-compile all the applications for them to work properly.Anonymous
March 29, 2010
I seem to remember reading something about the Java developers fixing this bug (yes, I consider it a bug) in JDK1.2 or so. I'm with the customer: I find it pretty horrific that this wasn't fixed. I understand the logic of being conservative about changing things, but this strikes me as dangerous. People who have the mental model that the customer has, are likely to use (and in fact I frequently DO use) the ability to override something as a sort of 'security wrapper' around the base class methods. I put security in quotes intentionally: I know it's bypassable if you have full trust or the ability to use reflection or whatever. It's a guard against doing something accidentally, to enforce invariants. For example, consider this in a world pre-generics: public class StringList : ArrayList { override Add(object o) { if (!(o is String)) throw new ArgumentException("parameter must be a string"); base.Add(o); } // and override other methods as well similarly } Suppose that somebody else inherited from a prior version of StringList in which the StringList developer forgot to override Add. Now they can add things into the StringList that aren't strings. Your approach means that the developer of StringList can't assume that his overrides will actually be called. Is that really the "conservative" approach?Anonymous
March 29, 2010
Couldn't there be some sort of attribute that tells the compiler that we want bravo to be inserted between alpha and charlie? Then we could expose this technique in situations where we expect our Type II development to insert method calls, while preventing exposure of other calls?Anonymous
March 29, 2010
The comment has been removedAnonymous
March 29, 2010
The customer-proposed behavior seems to fit better with dynamic languages to me. A lot of C# is about pushing as much verification as possible into the compiler, such as type analysis and deciding what method to call. As more work gets moved to runtime, you become more dependent on automated tests for correctness. In C#, you will almost never end up with a production bug because you didn't specify the right number of arguments or called a non-existent method, especially if you recompile against new versions of binaries. In a dynamic language, these things can happen easily without sufficient testing. So, while the current behavior may seem non-intuitive at first, I think it is the correct behavior for a static language like C#. I may start to feel differently depending on the direction which the new dynamic features in the language take.Anonymous
March 29, 2010
It seems like 'you' (the fake one having the fake conversation with the fake customer) actually want to be able to write a C# equivalent to 'this->Alpha::M()'. When you say "A base call gives me 100% of the safe, predictable, understandable, non-dynamic, testable behavior of any other non-virtual call. I rely upon those invariants to keep my customer's data secure" it's true only so far as you are actually sure what 'base.Bleck()' is calling. The only way to be sure in the current model is to read the source of the class that you are inheriting from and actually check whether the method is overridden or not. That seems a bit sketchy to me, but I can see where you are coming from: we generally expect things to get modified at the leaves, and test or design to that effect. So, time for new syntax and breaking changes, hurray!Anonymous
March 29, 2010
It seems like 'you' (the fake one having the fake conversation with the fake customer) actually want to be able to write a C# equivalent to 'this->Alpha::M()'. When you say "A base call gives me 100% of the safe, predictable, understandable, non-dynamic, testable behavior of any other non-virtual call. I rely upon those invariants to keep my customer's data secure" it's true only so far as you are actually sure what 'base.Bleck()' is calling. The only way to be sure in the current model is to read the source of the class that you are inheriting from and actually check whether the method is overridden or not. That seems a bit sketchy to me, but I can see where you are coming from: we generally expect things to get modified at the leaves, and test or design to that effect. So, time for new syntax and breaking changes, hurray!Anonymous
March 29, 2010
More to the point, if this your mental model: "I specifically wish Animal.Feed to be called here; if at runtime it turns out that evil hackers have inserted a method Mammal.Feed that I did not know about at compile time, I still want Animal.Feed to be called. I have compiled against Animal.Feed, I have tested against that scenario, and that call is precisely what I expect to happen. A base call gives me 100% of the safe, predictable, understandable, non-dynamic, testable behavior of any other non-virtual call. I rely upon those invariants to keep my customer's data secure." This is an example of the "it rather involved being on the other side of this airtight hatchway" attitude: You're running code linking to a different version of Bravo.dll! Unless the ONLY thing you do with Bravo.dll is make base class methods that are expected to skip it entirely and call through to Alpha, you're ALREADY in an untested scenario and you can't presume any of your invariants to hold. Use some kind of strong naming and assembly binding to guarantee that your code won't run if the version of Bravo.dll isn't exactly the one you tested against. The current behavior means that the author of Bravo.dll is getting HIS invariants silently bypassed without any ability to do anything about it. If you have to write code that presumes that any time you override a method your override might be silently skipped, that makes it really hard to enforce invariants!Anonymous
March 29, 2010
The comment has been removedAnonymous
March 29, 2010
I think that the real surprise here is not in the behavior of "base" per se, so much so as it is with the apparent mismatch between "base" and "override" semantics. If "override M" in C would have overridden A.M and ignored B.M here as well, an argument for consistent rule could be made: always determine those things out at compile-time. But this is not the case, so from a perspective of a developer who wants "100% of the safe, predictable, understandable, non-dynamic, testable behavior" - well, but you don't have that here already, either way! And the reason why a match here is important is that, in practice, "base" is used together with "override" - in an overridden method, to call a base implementation - 9 times out of 10, if not more often than that. So, for many developers, the mental model of what "override" means is defined in terms of "base"!Anonymous
March 29, 2010
The comment has been removedAnonymous
March 29, 2010
I'm not sure I can agree with you. As a matter of fact, you are the one who convinced me that you're wrong: http://stackoverflow.com/questions/2323401/how-to-call-base-base-method/2327821#2327821 It seems that you said that calling "base.base.M()" is illegal in C# because it could break the invariants of the base class. However today you say that I am stuck with "base.base.M()" even when what I meant was "base.M()" because it maintains my invariants. What good is maintaining my invariants if it breaks the invariants of my base class? How can I possibly expect to maintain my invariants if I'm doing something known to break the invariants of my base class? Well, at least now I know how to call "base.base.M()". I just have to compile against a version of the base class that doesn't override M, and then run against the real version of the class that does override M.Anonymous
March 29, 2010
I'm with the customer on this one too. This is quite contrary to my expectations. As others have said, I would expect to be able to switch and change existing DLLs and have related code automatically be updated without re-compilation. More fundamentally, though, I would expect the compiler to honor my virtual code, even if it thinks it knows better. Such optimizations belong in the runtime, not baked into the compiled IL. I now fear that I will need to review many virtual calls at the IL level and possibly patch the IL... I used to manually compile variance code before C# 4 which now supports it. Now I may need to do this on virtual calls... maybe I should just do all of my coding in IL. This is terrible. Please fix it.Anonymous
March 29, 2010
The comment has been removedAnonymous
March 29, 2010
The comment has been removedAnonymous
March 29, 2010
Another way to look at it: overriding a method with a body that just forwards the call to the base class, without doing anything else, should be a null operation. If overriding a method changes the way the code links, such that virtual method calls get skipped on their way up the hierarchy, then something is grievously broken. You might recall the argument against default parameters in C#: the problem being that they bake the constant value of the default into all the call sites, harming version resilience. What you are arguing for in this post is to make the hierarchy, and the set of methods that ancestors it overrides a constant. You are arguing for baking in the entire history of a method's override chain all the way up to the ultimate ancestor at every base.Method() call, as a hard-coded constant.Anonymous
March 29, 2010
Interestingly, the ECMA CLI spec's description of the "call" opcode (section 3.19 of Partition III) includes this text: "If the method does not exist in the class specified by the metadata token, the base classes are searched to find the most derived class which defines the method and that method is called. [Rationale: This implements“call base class” behavior. end rationale]" This appears to me to indicate that the CLI spec writers agree with your customer about the anticipated behavior of base calls...Anonymous
March 29, 2010
Describing the customer's expected behaviour as an entirely new kind of pseudo-virtual call is a bit over the top, too. It's the normal behaviour in almost all OO languages. In which OO languages does this behaviour happen without recompilation? Java, apparently. Are there others? -- Eric You also assert that "no C# programmer in the world has ever designed for" it - but I would assert exactly the opposite, that almost every C# programmer designs for the customer's expected behaviour.Anonymous
March 29, 2010
The comment has been removedAnonymous
March 29, 2010
I, too, am with the customer on this one. I think barrkel said it best: the idea of calling "base.M()" (in the minds of most developers) is that you're passing the call to M() up the chain of inheritance, stopping at the first one you find (from a runtime perspective, not a compile-time static analysis perspective). While I haven't yet encountered this issue, I know that I would have been utterly confounded by the fact that the compiler exhibited this behavior. It sounds like the only short-term solution is to insert override stubs for functions that might get overridden in the future.Anonymous
March 29, 2010
For what's it worth, all languages that target .NET seem to match C# behavior - checked with VB and C++/CLI.Anonymous
March 29, 2010
I have to say that like many other commenters, I'm on the customer side on this one. My mental model has never been "base.M() == Alpha.M()" but rather "base.M() == "Bravo.M()" wherever that method definition may come from (Alpha actually)., although I now know it's wrong. But seriously: who would naturally assume that "base" may statically mean "base.base" ? When you write base.M() how can you know which method you are statically calling ? Do you think people actually go and check at which level in the hierarchy M is defined ? No, they surely don't (especially with deep class hierarchies, e.g. WPF). When you write base.M() you actually think: "call the default behavior" before, after or during some custom processing you're putting in place. Stuart's StringList is a good example of this. I also agree this makes the whole framework fragile. You can't fix bugs in .NET by releasing a service pack if that means adding an override in the leaves of the class hierarchy, as Daniel Grunwald noted. This really looks like something that ought to be fixed, in my opinion. Note that the "fix" only needs to be applied to cross-assembly base calls. Every call to a base method defined in the same assembly can be kept as a non-virtual call (which would probably be preferrable for performance, although the lookup only needs to be done once, as class hierarchies don't change at runtime.)Anonymous
March 29, 2010
I'm with the customer on this one. Your concerns over ensuring only the code you compiled against is run is moot, as that can be handled numerous other ways. The ability to dynamical change out Bravo may be needed in certain environments and the compiler is clearly failing to do so. M2C.Anonymous
March 29, 2010
I agree with the customer as well. I'm actually quite surprised by this behavior. I thought the call was a dynamically determined call to the nearest matching method. I think this is something that needs to be changed, perhaps the same way Java did it by keeping it backward compatible.Anonymous
March 29, 2010
This is just plain broken, sorry. Definitively a bad call. Let me explain why: Let's say there wasn't just M() that was being used, but also P(). However, P() was overridden in Bravo and thus is being invoked indirectly by the override in Charlie. Now, a new assembly is deployed that has changes in M() and in P(). However, only HALF of these changes are being picked up!!! Think of M() and P() being called Add() and Remove() instead. Now imagine you added a counter to how many items are added and removed. Suddenly, your application shows you that either too many items have been added or removed, when in fact everything is balanced. Again, because only HALF of the implementation is being used. I'm literally shocked that anyone would consider this the proper behavior of a virtual base method! Please fix it.Anonymous
March 29, 2010
I'm also agreeing with the customer on this one. I find the scenario far more interesting though if we don't assume that I "own" all of the DLLs or classes in question. Assume class Alpha and class Beta are provided in some third-party library, say some framework like Winforms or ASP.NET where there is a concept of a "control". It's natural in such type heirarchies to inherit, override certain "OnXXXX" methods where you are also expected to call the base member. (for example Alpha = System.Web.UI.Control, beta = UserControl, and Charlie = my class) Now suddenly, if the framework needed to fix a bug which required the addition of this method to the middle class, they can't, since the only way they could do this was to ask everyone in the world to rebuild their Charlie classes. I'd hate to see what "fun" would be had by all involved if this bug had security ramifications! Eric> "I am willing to give up safety, predictability, and the knowledge that what runs on my customer's machines is what I tested." It seems like you're trying to defend this customer against their customers going "oh, well, I changed your assemblies by copying a few random ones into place, as well as one I built myself. Hey, your app doesn't work now. What's gives?" There's a reason why most pieces of hardware have a ltitle "warranty void if broken" sticker. :-) Somebody's point of "it's rather involved being on the other side of this airtight hatchway" seems to defeat any argument this is being done to help security. Sadly, I might have to admit this is one of those sad "meh, it's not good where we are but we can't change it" times. :-(Anonymous
March 29, 2010
The comment has been removedAnonymous
March 29, 2010
The comment has been removedAnonymous
March 29, 2010
IIRC the methods-are-virtual-by-default style resulted in a good deal of headache for Java as more libraries came online. JDK library methods were often inadvertantly hidden by the user of a 3rd party library because the 3rd party library unintentionally hid the JDK library method. You could spend hours trying to figure out why a method call wasn't working the way you expected only to find out you weren't calling the method you thought you were. I'm all for convenience but if you want to change how a method is resolved without recompiling against the shipped library you can use assembly redirection. You still have to compile against the redirected-to libraries but IMHO that's a good thing (for all the reasons Eric mentions).Anonymous
March 29, 2010
The comment has been removedAnonymous
March 29, 2010
+1 for another programmer who thought the customer's mental model was right. That being said, dynamically swapping out base classes is risky business, and I would not be surprised if it bit me. You admit in your article that there are two equally valid mental models, but imply that your belief is that most programmers expect the static model. Based on the limited sample of your blog comments, that belief appears to be incorrect. Would you reconsider your opinion if, for sake of argument, the compiler actually was surprising the vast majority of your developers. All that being said, fixing this doesn't even come close to meeting the -100 point barrier for me. Spend you time on a decent metaprogramming system.Anonymous
March 29, 2010
Unlike most of the commentors, I actually agree with Eric on this one. The customer controlled all 3 dlls, there is no reason not to distribute the recompiled Charlie.dll with the recompiled Bravo.dll. Compilers aren't some big scary program (on the usage side of things), you just hit CTRL+SHIFT+B and magically dlls spit out in your output directory.Anonymous
March 29, 2010
@Robert: "The customer controlled all 3 dlls, there is no reason not to distribute the recompiled Charlie.dll with the recompiled Bravo.dll" That may be the case in this scenario but it's certainly not always the case. If you're the developer of a library, do you necessarily know to tell all your consumers that they have to recompile any time your library is updated? How are you supposed to patch bugs in your library if you can't get every customer to recompile their code? Does .NET document rules for binary compatibility? As in "if you limit your changes to adding new methods and classes and ... and ... and ... then existing code will work without recompilation"? I've always presumed "adding or removing an override to a virtual method" to be one of those binary-compatibility-preserving operations. Apparently it isn't.Anonymous
March 29, 2010
@Stuart - no matter the language or operating environment, experience has told me, when in doubt, recompile. If I upgrade to a new version of a library, I'm going to recompile and test, and I would expect the same of people who upgrade to the latest and greatest of any library I wrote to do the same. Recompilation is cheap.Anonymous
March 29, 2010
Even if the object semantic is broken, it's quite natural for a statically-checked language to make this optimisation. Findind at runtime which base method should be called would slow down each and every virtual call. Where I work, when we ship hotfixes, we just follow the rule of always delivering a fresh Charlie.dll when just Bravo.dll has changed. This ensures we can never run into this problem.Anonymous
March 29, 2010
Eric, I must with you. Especially when there is such a simple way to get the customer’s expected behavior... public class Bravo: Alpha // In Bravo.DLL public override void M() { base.M(); } } ... if the above code was in Bravo before Charlie was compiled, then the virtual method call would work as he expected.Anonymous
March 29, 2010
The comment has been removedAnonymous
March 29, 2010
The comment has been removedAnonymous
March 29, 2010
Adam Robinson wrote: "While I haven't yet encountered this issue, I know that I would have been utterly confounded by the fact that the compiler exhibited this behavior." And I would have to say, I would completely expect this behavior (oddly enough)...although I've never had to deal with it. Then again, I look at it from the point of view of having toyed around with building my own compilers in the past. To me, this sort of falls in the category of "it's better to be safe and recompile" rather than assume anything about how things will be handled under the hood.Anonymous
March 29, 2010
I'm with the customer too. If M is virtual, I always thought that base.M() is a special virtual call that skips the implementation in the current class. If M is not-virtual, I expect it to be resolved statically. So, if someone adds a non-virtual M in Bravo, I'd expect the call in Charlie to still resolve to Alpha. In any case, newly introducing a method in Bravo that hides a non-virtual base class method although B is not sealed and has already been derived from sounds to me like bad programming practice. It would only be legitimate if it can't break anything.Anonymous
March 29, 2010
With respect to your update, Eric: > How is that situation made any different from the customer's perspective by making the non-virtual method call a base call instead of an ordinary non-virtual method call? Surely what is good for some non-virtual calls is good for all non-virtual calls, no? I think the point of contention is that base.M() is not obviously a "non-virtual call" - in fact, those opposing it would rather want it to behave like virtual. This is even explicitly mentioned in some of the comments above. The informal definition of base-calls that many (most?) programmers use is "call the method of the nearest base class". More formally, this would amount to "do a virtual call, as if the type of the object was that of the immediate base". So, as a question, it is, perhaps, narrower than it really should be - it makes certain assumptions that are themselves contested by those who'd answer it differently from you.Anonymous
March 29, 2010
"Just recompile" is not an answer in general, because all 3 assemblies may come from different vendors. Don't forget that Charlie may also be declared in some library which your code is using, and for which no source code is provided.Anonymous
March 29, 2010
The comment has been removedAnonymous
March 29, 2010
The comment has been removedAnonymous
March 29, 2010
Quick extra note: some people noted that this is an optimization; if so, that optimization can still occur at assembly load time; there's no need for the compiler to bother with it and you still retain the lower cost of non virtual dispatch at runtime. I'm also curious how fixing this bug is supposed to break existing code - It strikes me as very odd that it's even possible to call Alpha's M() without Bravo's consent - that's certainly not easy (or even possible?) to achieve normally on a virtual function like that - so it's very unlikely there's any code relying on that very hard-to-trigger behavior - right?Anonymous
March 29, 2010
To all of those who say: "recompile to be safe" - there are scenarios where you cannot recompile.
- Our application has a plugin model. Most plugins are rarely updated compared with our application, so it is important for us to allow old plugins to run in new application versions. We don't have any source code for these plugins available. So we try to keep our application binary compatible with previous versions. This issue means for us that we are unable to ever add any overrides in any non-sealed class. This includes quite a lot of user controls (WPF and WinForms) where adding overrides to existing methods is considered normal. Plugins also overriding these methods expect "base.OnEvent(...)" to mean "handle that event the usual way", not "bypass the app's usual handling and call WPF directly, breaking the app's invariants".
- We're producing a WPF control library. It runs fine on both .NET 3.5 and 4.0. However, as we've learned in this post, using a .NET 3.5 compiled library may break WPF's invariants by calling the wrong base method (unless the WPF team avoided adding any overrides) - so from now on, we and our customers have to deal with maintaining two separate builds where one could suffice if this issue was fixed. In summary, this issue makes binary compatibility very, very fragile. I thought binary compatiblity of assemblies way an important feature of C#, but it appears I was mistaken :(
Anonymous
March 29, 2010
While I have tremendous respect for Eric, I have to agree with the majority of those who have commented here that the behavior is unexpected and probably undesirable (at the very least violating the principle of least surprise). First, is this behavior consistent with the spec, particularly the last paragraph of §7.5.8: "When a base-access references a virtual function member (a method, property, or indexer), the determination of which function member to invoke at run-time (§7.4.4) is changed. The function member that is invoked is determined by finding the most derived implementation (§10.6.3) of the function member with respect to B (instead of with respect to the run-time type of this, as would be usual in a non-base access). Thus, within an override of a virtual function member, a base-access can be used to invoke the inherited implementation of the function member." Second, the "evil hackers" argument seems like something of a red herring. As you yourself stated in a previous entry: "When you call a method in Smith and pass in some data, you are essentially trusting Smith to (1) make proper use of your data[...], and (2) take some action on your behalf. " If Bravo is compromised yet we trust it sufficiently to derive from a class defined within it, we already lost, no? Besides, either the assembly is strongly-signed and is trusted (in which case we trust it to not be evil) or it isn’t (in which case nothing is stopping evil code anyway). Third, as an author of class C, if I call base.M() should I care if (or do I even have any means to know) whether M() is immediately defined by the parent class? By not defining trivial overrides (protected override void M() {base.M();}), is B explicitly abrogating its ability to override virtual calls to M from derived classes? Fourth, the key difference between the example the compiler faced and the counterexample you defined in the epilogue is that of M() being a virtual method. I think it fair as a developer to expect that a call to a virtual method can change at runtime to an implementation defined by a subclass, and to adequately prepare for this possibility. For a non-virtual class, I would probably expect a call to (this.M()) to equate to the compile-time equivalent (in this case, ((Alpha)this).M()). It would seem that by calling a virtual method to begin with, we are already, as you state, "giv[ing] up safety, predictability, and the knowledge that what runs on my customer's machines is what I tested". If this is unacceptable, why would we swap out the DLL for a base class in this fashion to begin with?Anonymous
March 29, 2010
I was going to post a response to your update but Pavel's post at 11:06am that starts "With respect to your update, Eric:" says almost exactly what I'd want to say. To elaborate slightly: In my mental model, a base call is not a non-virtual call: in fact, my mental model of C# doesn't include a concept of a "non-virtual CALL" at all. My mental model is that non-virtualness applies to methods, not to calls. So in the scenario in your update, you are making a call to the non-virtual method Alpha.M(). In the original scenario, you're making a call to the virtual method M(). The "base" prefix has a special meaning that indicates the virtual lookup should proceed from the immediate base class of the current class, but it doesn't negate the "virtual"ness. It doesn't turn a virtual method into a non-virtual method, just changes where the virtual lookup starts from. And "base" refers to the base class of the caller, which is to say that "base" means Bravo, not Alpha.Anonymous
March 29, 2010
The comment has been removedAnonymous
March 29, 2010
I'm going to have to go with Eric's side. With the given example we're dealing with strings and it's clear that the method in question is incorrect for both B and C, since the string returned shows the inheritance heirarchy. In a more complex example though, you might have a calculation that for the incorrect version of B is correct for C. So when fixing B you would actually break C. Regardless of whether or not another company makes C, it isn't safe to assume that fixing B also fixes C, and it would be as likely that fixing B now breaks C. We're not talking about tweaking the code. We're talking about adding a completely new function (on the level of B).Anonymous
March 29, 2010
The comment has been removedAnonymous
March 29, 2010
As Ayende has said a while ago, when most customers have a different mental model than the Microsoft team, it's the Microsoft team that should change how things behave. Being Ayende, he actually got the MVC team to make that change :) Note: this is the second time I see Eric have a "weird" mental image of what should happen; the previous one was regarding static virtual methods (also known in Delphi as class virtual).Anonymous
March 29, 2010
@Mark: > I'm going to have to go with Eric's side. With the given example we're dealing with strings and it's clear that the method in question is incorrect for both B and C, since the string returned shows the inheritance heirarchy. In a more complex example though, you might have a calculation that for the incorrect version of B is correct for C. So when fixing B you would actually break C. This interpretation is inconsistent with proper use of virtual & override. The class that introduces a virtual method specifies the contract for that method, which all derived classes are expected to adhere to. If they do not, they violate LSP, since a client could call the override via a reference to a base class type without even knowing it; he has the right to expect the contract to be upheld regardless of the effective type of the referenced object. Consequently, whether B overrides A.M or not, the implementation of C can expect that base.M - regardless of what it ends up calling - will uphold the contract for A.M. If the newly introduced B.M does not do so, it would break far more than this particular edge case - it would break any client code that makes virtual calls to A.M in a situation where B may be the effective type of receiver.Anonymous
March 29, 2010
Just to make the distinction even clearer: If the new version of Bravo looked like this: public class Bravo : Alpha { public new void M() { ... } } ie "new" instead of "override"... THEN I would expect that Charlie's base.M() should still end up calling Alpha.M() instead of Bravo.M(), until Charlie got recompiled.Anonymous
March 29, 2010
The comment has been removedAnonymous
March 29, 2010
The comment has been removedAnonymous
March 29, 2010
I would like to share my two cents. Before I do, I will say that a strong case can be made for both positions of this issue. Furthermore, it's entirely unclear what the appropriate or most desirable behavior should actually be, and given that there are potentially significant consequences to the large body of deployed code already shipped, a lot of thought (by folks smarter than me) needs to take place before such a change should be made. That said... After reading the article and your (Eric's) explanation of what happens, I have to say I was surprised. My "mental model" leaned in the direction that the customer, and many other responders, have described - namely that base calls in a virtual method send the message up the inheritance chain, and don't skip directly to the base version that existed at compiled time. Second, similar to the topic of how the stack works in .NET/C#, I've always treated the mechanism of the base call as being an implementation detail - an abstraction that I didn't have to worry about. However, in this case, it turns out to be a leaky abstraction - you have to know a great deal about how the compiler wires a base call to be able to design and implement inheritance hierarchies that behave correctly - particularly in special cases like runtime substitution of compiled code. I have to say that I find that a bit unsettling because the vast majority of C# developers likely do not have this level of understanding about how their code is translated into IL. Third, I now realize that there are potentially a large number of cases where this type of problem can be introduced. Take, for example, the .NET framework itself. It's quite common to inherit from classes in the .NET BCL - these class can, in turn, inherit from yet others within the BCL. Now imagine a case where some user-defined class inherits from a version 2.0 .NET class - but at runtime, the compiled code is loaded into a different version of the CLR than what the code was originally compiled against (there are numerous cases where this actually happens in real world code) and dynamically bound against different versions of the BCL assemblies. It's entirely plausible that the BCL code itself has added some overrides that may have previously been omitted. Consequently, at runtime the behavior continues to call the wrong overload. Another example would be service releases of the .NET framework itself, which could introduce overrides of calls within the inheritance model that didn't previously exist. Most shipped applications are not going to get recompiled when a service pack is deployed ... nor in most cases is that even possible. It seems that the authors of the .NET BCL will have to be careful in the future to be aware of such impacts. Fourth, I don't view base calls and overload resolution as the same thing. In the update to the article, you mention that swapping in a M(int) override in Bravo would exhibit the same type of problem as the virtual method's base call does. Fifth, I am surprised that this should be such a rare case. It would seem to me that this behavior should occur more frequently than it does. The fact that this isue hasn't seen been more common and impactful is hard to reconcile with my intuition about the frequency that should a change could be introduced and the amount of existing, compiled code out in the real world.Anonymous
March 29, 2010
The comment has been removedAnonymous
March 29, 2010
@Pavel: I see your point, I hadn't thought that far into it, but after posting I was trying to come up with an real-world example to what I was saying and couldn't come up with anything that wasn't hacky at best. With that said, I think I'm swaying sides a bit on this, though I think Leo said it well that it's unclear what's really desirable. When it comes to why this hasn't been seen more often with releases of .NET, I would assume it has to do with the assembly manifest pointing to the specific version of .NET an assembly was built against. I don't completely know what the default is for this though.Anonymous
March 29, 2010
The comment has been removedAnonymous
March 29, 2010
The comment has been removedAnonymous
March 29, 2010
To commenters:
- Whether you agree or not, other people may depend on the current behavior without even realizing it. They would be quite surprised if their code stopped working and required them to change their entire inheritance model.
- There are many other ways to accomplish the desired behavior
- Do you really want your code to depend on such a subtle and misunderstood detail? Wouldn't it be better if such an important architectural detail were spelled out explicitly?
- If you really can't recompile, see point 2
- To those of you who say Microsoft should change to match the customer's mental model, well, other customers may have different mental models than you. Please don't break my code because you think yours is more important.
Anonymous
March 29, 2010
Eric wrote: > One of these days I need to sit down and just read all five hundred pages of the C# 1.0 and 2.0 design notes. And share all the interesting parts with us in a lengthy series of blog posts, I hope. :)Anonymous
March 29, 2010
The comment has been removedAnonymous
March 29, 2010
Couldn't this unusual possibility also occur? public class Bravo: Delta // In new Bravo.dll, now pointing to a new base class { public override void M() { Console.WriteLine("Bravo"); base.M(); } } ...where Delta looks like... public class Delta { public virtual void M() { Console.WriteLine("Delta"); } } Now what happens?Anonymous
March 29, 2010
Eric, This has been a fascinating discussion. Is there any way to share these design notes? I am sure these notes would be far more interesting read than the spec. I know, I know, you will not be able to share those design notes, but it wouldn't hurt to ask, right? :)Anonymous
March 29, 2010
I see the problem with the update. And I see why it's not a problem for Java - there's no "new" or "override" keyword so it's impossible for a method inserted into the hierarchy with the same name and signature NOT to be an override. I'm not sure I agree with the tradeoff; I'd rather have a crash in the case where someone introduces a new private method with the same name as an override, than have the wrong behavior silently succeed. But at least I understand the reason for the tradeoff. And it does, indeed, seem to be one without a good solution, unfortunately. I'd argue for the new CLI opcode, but obviously the C# team tried that and failed. Perhaps a compiler option to switch to calling Bravo.M() even if it meant that certain kinds of changes to Bravo might cause the code to crash? To my mind, "certain kinds of changes to Bravo might cause my code to crash" is better than "certain kinds of changes to Bravo might cause my code to completely ignore all the invariants that the author of Bravo was trying to enforce, but still continue to run without giving any error messages". The only other solution I can come up with is to have a way for the compiler to automatically insert a do-nothing stub for every non-overridden base class virtual method, equivalent to override M() { base.M(); }. But that'd enlarge the code quite a lot and mostly unnecessarily.Anonymous
March 29, 2010
Eric, I am always impressed by the thoughtfullness and thouroughness of the C# compiler team - the fact that they identified this issue in the early days is impressive - and the complexity of the problems that have to be coordinated between the CLR team and the language teams is also remarkable. The insight this kind of discussion provides into language and runtime design is wonderful, thank you. One question that occurs to me though. When adding a private new M(), why can't the compiler also generate an overriding M() that pass-through calls base.M()? Presumably this would allow the CLR at runtime to correctly select between the two. I understand that normally (in user-written C# code) you can't both override a method M() and supply a private method M() directly, but what prevents the compiler from being able to do so? Presumably the private/override versions of M() are placed in different "slots", yes?Anonymous
March 29, 2010
If I understand the behavior correctly, it used to work as most of us expect. However, swapping out the Bravo assembly with one that has public class Bravo: Alpha { new private void M() { Console.WriteLine("Bravo"); } } would cause your program to crash unless you recompile. Then they fixed it to make that scenario work, but broke the scenario where a maintenance release (fixing bugs, patching security holes) adds an override to a class that didn't have one before. So instead of an extremely unlikely situation breaking my program and crashing, an unusual, but still less unlikely, situation silently breaks my program's invariants with no way to debug? I think I like the old behavior better. I don't see "new private" methods as something you add to a class; I see them as something you already had in your class, but you had to add the "new" only because a base class was modified to include a new method of the same name. If I'm the author of Bravo class, and I already know that my base class Alpha has a method M, why would I create a new private method M? Unless I really wanted to confuse all maintainers of the class, I would pick a name for the method that doesn't have the same name as an unrelated method of the base class. Am I missing something here? How often does somebody add a new private method to a class that has the same name as a virtual method of a base class?Anonymous
March 29, 2010
Aren't all the calls we're discussing made using a particular mdtoken representing the target method, and not by name? So I'm a little surprised that there exists an mdtoken for B::M() when B didn't override A::M(), and I'm doubly surprised that the metadata token which represents that potential B::M() is structured in such a way that it could match "B::new M()" instead of "The implementation of A::M() in class B" which clearly is that sort of "truncated search virtcall" that fixing this problem requires. In fact, use of Reflection.Emit certainly led me to believe that the metadata token was of the latter style. In fact, it sounds like the CLR could provide the desired behavior without a new instruction. I propose that if you upcast arg.0 and then do a virtcall, you ought to get the "truncated search". Currently you'd do this to call the most overridden method prior to a "new" method, it seems like it shouldn't break any existing code to define the truncated search if this is used in the absence of a "new" method (and then it continues to work if a "new" method if added by an update to the assembly defining an intermediate parent class).Anonymous
March 29, 2010
> Whether you agree or not, other people may depend on the current behavior without even realizing it. They would be quite surprised if their code stopped working and required them to change their entire inheritance model. Note that this works both ways - there may also be existing code which is written using the "wrong" mental model, and is therefore buggy, without people who wrote it knowing about that.Anonymous
March 29, 2010
@Ben > Aren't all the calls we're discussing made using a particular mdtoken representing the target method, and not by name? So I'm a little surprised that there exists an mdtoken for B::M() when B didn't override A::M() Have a look at Ecma-335 describing "call" instruction. I had cited the relevant paragraph in an earlier comment, but here it is again, for convenience: "If the method does not exist in the class specified by the metadata token, the base classes are searched to find the most derived class which defines the method and that method is called. [Rationale: This implements“call base class” behavior. end rationale]" > and I'm doubly surprised that the metadata token which represents that potential B::M() is structured in such a way that it could match "B::new M()" instead of "The implementation of A::M() in class B" There is no difference between the two. On MSIL level, a non-newslot method named M will override any method M with a matching signature inherited from a base class. Thus, a token for B::M always references M in B, whether it overrides anything or not. .override is a different thing, but it's orthogonal to all this. > In fact, it sounds like the CLR could provide the desired behavior without a new instruction. I propose that if you upcast arg.0 and then do a virtcall, you ought to get the "truncated search". Currently you'd do this to call the most overridden method prior to a "new" method, it seems like it shouldn't break any existing code to define the truncated search if this is used in the absence of a "new" method (and then it continues to work if a "new" method if added by an update to the assembly defining an intermediate parent class). It seems to me that it would break any existing IL that presently contains no-op upcasts on ldarg.0. While the upcasts are meaningless, such IL is perfectly valid, and has a well-defined meaning alread.Anonymous
March 29, 2010
Hrm... so the behavior was as a lot of us expected, but a really obscure case would crash the CLR. So a breaking change to C# compiler was made in 2003. I really don't see how this would have been a breaking change in the CLR. After all, a compiler would still be free to make any sort of call they wanted. It would really be a "do a virtual dispatch call on object X, treating it was type Y for method lookup". Could be a useful instruction in other cases. It seems that would have been a not to difficult and fairly safe addition to the CLR, in order to prevent a breaking change to the compiler. Weird.Anonymous
March 29, 2010
The current compiler behaviour can still cause runtime crashes. Suppose you have four classes in the hierarchy. Insert an extra class Aleph in the hierarchy between Alpha and Bravo and give Bravo an override of method M. When Charlie is compiled the call to M goes to Bravo. In a future update Bravo loses its override of M and, at runtime, the call to Bravo.M ends up at Alpha.M and everything still works. But then another update comes along adding a private method M to Aleph. And the code crashes at runtime. But maybe this is a little far-fetched. P.S. I know the blogging software isn't your problem, but the captcha image doesn't appear in FireFox. Since it's a JPEG with an aspx extension I'm guessing the server isn't sending the correct mimetype.Anonymous
March 29, 2010
This has been a very interesting discussion, and I am suprised by the number of people who have not been aware of this. Obviously, the majority of readers (or at least posters) are not aware of the basic tenet: "If you develop a NON-SEALED class, you MUST test the implementation via (often multiple) derived classes." To minimize the impact, one technique is to override every virtual method that you may possibly want to change the implementation at a later day with a simple call to base. [We actuallly do this via our automatic "class setup" wizard].Anonymous
March 29, 2010
@carlos: > in a future update Bravo loses its override of M Well, removing members has always been considered a versioning-breaking practice, so this isn't anywhere as surprising as Eric's original case (which is merely adding a non-virtual member).Anonymous
March 29, 2010
The customer is right, Eric is wrong. Eric's solution breaks separate compilation, and it breaks the simple OO delegation model, both of which are unacceptable IMO. I'm also surprised that it would be so difficult to implement this in the CLR. All the metadata required to resolve the base class call is available to the VM in the CIL. If derived classes indeed cannot access private members, then that member should not be in the publicly accessible vtable, so resolution isn't a problem. If it must be in the vtable for some odd implementation reason, then add protection information, ie. private/protected, to the slot resolution process so it skips protected members.Anonymous
March 29, 2010
This is the basis for a great new interview question: What are the semantic differences between classes Bravo1 and Bravo2? Which class definition is better and why? class Bravo1 : Alpha {} class Bravo2 : Alpha { public override void M() { base.M() } }Anonymous
March 29, 2010
Eric, With all due respect, I think that you are wrong. This behavior is quite disturbing. So much that I would even delay VS2010 just to fix it, although I know that it is not realistic. (SP1 would do fine.) I am rather concerned about the affect that this issue may have on my code. I agree with most of what the other commenters are saying here, so there is little need to repeat. Request: At the very least, you and your team need to sit down and really discuss this issue in depth. Please feel free to solicit us for more discussion. I am sure many of here feel passionately about this issue and would like to be part of the discussion / solution.Anonymous
March 29, 2010
"if you change a base class then you should recompile your derived classes" That doesn't feel right. My base classes exist as a rock for me to derive reliable, predictable and very well understood behavior from; they help me form version 1 of my application and when version 2 comes along they are often the favourite candidates for extension. Some of these base classes are used outside of the core team and are deployed in applications that are not interested in the new functionality, but would like the latest base classes for the various fixes and rewrites in the classes and methods they actually do use. They aren't very interested in rebuilding redistributing their derived classes though! In essence, how does the quoted sentence stack up against design by base class vs design by interface? The whole "Framework Design Guidelines" angle is what I'm lining up against here. Suggesting that assemblies containing derived classes should be recompiled when the base class changes doesn't feel practical in many, by no means alll, or indeed most, but certainly a significant number of, situations.Anonymous
March 29, 2010
Two separate things First of all: Regarding Eric's "being wrong" There are two solutions I can think of.
- Runtime check on every "base" call made ever. Every single one. At that point, you have successfully mitigated the purpose of having compiled dlls in the first place. You have carefully avoided the benefits of compilation, and in order to fix a very obscure scenario.
- All classes which inherit from classes with virtual methods ANYWHERE in their heirarchy (this is all classes, by the way) will automatically generate public override VirtualFunction(params) { base.VirtualFunction(params); } That would solve the issue, but again, when you call ToString() on anything, you would end up with a very tall stack trace, which would obviously have performance implications. This would have a measurable impact on ALL programs.
- Check if the reference dlls have been modified When check? Store the information where? What do you do if it HAS been modified? Dynamically recompile my own dll? What if there are irrelevant breaking changes? This solution is impossible. Ultimately, Eric's solution is the most pragmatic. Secondly, If after adding public override void M() { Console.WriteLine("Bravo"); base.M(); } to Bravo, which doesn't get executed, you then choose to SEAL method M, it will cause a runtime error, why is that?
Anonymous
March 29, 2010
I'm surprised that the opposition to this appears so strong. While I understand the concerns that some people have about libraries they don't have source for and particularly plug-ins, those are inherently risky situations anyway. If they break they break; and you should be prepared for it as part of your risk assessment. You don't have to be happy about it, but you should be prepared. I don't think those concerns should override common sense; by default, compiled code should not change its behaviour when a change like this occurs - when completely unknown code is inserted where no method existed before. That should only occur after the new method's been explicitly approved through recompilation (and retesting, etc.).Anonymous
March 29, 2010
Gavin, the problem is that this breaks silently. Breaking explicitly is always better than breaking silently. Security example: method M() validates access to an account. Alpha.M() checks some conditions. Later on, Bravo.M() adds another layer of checks - WHICH IS IGNORED. In what way is this a good thing?Anonymous
March 29, 2010
The comment has been removedAnonymous
March 29, 2010
I wonder, what behavior the customer would expect, if Bravo were modified as follows: public class Bravo: Alpha { public void M(params int[] x) { Console.WriteLine("Bravo"); base.M(); } }Anonymous
March 29, 2010
I've been thinking about the issue a bit longer and I think I have changed my mind...the customer is right. The main reason is that the code is "failing" silently to do what the coder expects. The obvious argument is that the coder should know how the compiler works but one of the main tenents of the c# team has always been to create a language that does what one expects. And seeing the vast majority of posts here, the compiler is failing big time and doing what no one expects silently. Eric uses security as an argument but plenty of posts have also argued that the current implementation can be as insecure or even more. If Bravo was to implement a new layer of data access security, the client could actually be fooled into a false sense of security as the new implementation in Bravo is ignored. Obviously when all dlls belong to the same developer the current implementation isnt a big deal. But when more than one developer is in the mix, things dont seem to work as well.Anonymous
March 29, 2010
The comment has been removedAnonymous
March 29, 2010
As a long time C++/recently turned C# dev I find this somewhat assuming. In C++ there is no base/super keyword and so you will likely write Alpha::M() - although you could write Bravo::M(). A common reaction to the brittleness of this (with respect to changes in the class hierarchy) is to typedef up an alias so you only have to change one thing when the class hierachy changes:- private: typedef Bravo base; [See http://stackoverflow.com/questions/180601/using-super-in-c] Thinking about how this actually behaves in C++, Eric's point make sense, but when considering the reason why this idiom is employed in C++ I think it matches the mental model of the majority here.Anonymous
March 30, 2010
> As a long time C++/recently turned C# dev I find this somewhat assuming. In C++ there is no base/super keyword and so you will likely write Alpha::M() - although you could write Bravo::M(). Two things of note here. First, there is __super available as an extension in VC++. Second, in the example that Eric gives (rewritten for C++/CLI), even if you use Bravo::M(), the actual call compiled by VC++ to MSIL will still be to Alpha::M().Anonymous
March 30, 2010
> There are two solutions I can think of. > 1) Runtime check on every "base" call made ever. Every single one. You miss one obvious solution: a single check at JIT time. It really only has to be done once per class load, because the entire inheritance chain for a given class is known at that point, and thus it is possible to determine, for any hypothetical "basecall" instruction in a body of a method of the class, which method it actually refers to. Once thus determined, it won't change later at runtime.Anonymous
March 30, 2010
Suppose you have a bug fix you inserted into Bravo, that makes Charlie work correctly. Charlie was created by the Type 2 team at a distant location, or there could be several different versions of Charlie created by various type 2 teams around the world. Are you saying that all of them have to recompile their Charlie because we added a bug fix implementation in Bravo?Anonymous
March 30, 2010
The comment has been removedAnonymous
March 30, 2010
The comment has been removedAnonymous
March 30, 2010
> just pick the best one - whichever one happens to be "most derived" in some future version of the world - by doing that analysis at runtime I propose we refer to this performance penalty as an "extra base hit".Anonymous
March 30, 2010
I can see the arguments for both sides, but I tend to agree with the customer (and the majority of comments). Having to recompile every bit of .NET code every time there's a service pack or security patch is not an acceptable option; neither is manually inserting no-op overrides of every virtual method from your base classes "just in case". If Bravo v2 breaks Charlie v1, you'll always need to recompile. With the pre-2003 compiler, non-breaking changes wouldn't require a recompilation; with the current version, they might, and you have no way of knowing until things start breaking in interesting ways. If the base method call works 99% of the time when Bravo.dll is replaced, and crashes 1% of the time when BravoCorp do something stupid, surely that's better than not working quite as you expect 99% of the time and the other 1% also not quite working as you expected (but not crashing). In other words, if there's something wrong, it's better to crash than to do the wrong thing.Anonymous
March 30, 2010
The comment has been removedAnonymous
March 30, 2010
I read the article but skimmed through the comments so if this question has already been asked I apologize. What happens if the order of operation is changed just a little. What if M exists in Bravo at the time that Charlie is compiled. Then M is removed from Bravo, recompiled (without compiling Charlie). I tried this and got Alph / Charlie once I recompiled Bravo. Is this the correct behavior? If this is correct then the scenario the "customer" wants can be done, through a strange work around but it can be done.Anonymous
March 30, 2010
The comment has been removedAnonymous
March 30, 2010
The comment has been removedAnonymous
March 30, 2010
The question is where the “middle” is. Is it within MSIL, or JIT/NGEN? An object using polymorphism has a hidden “vtable”. When to bind the vtable to the object is the key to confront both sides. Eric side wins if people agree the vtable should be build when C# compiler is compiling code. Eric side loses if people agree C# complier just making the MSIL. JIT or NGEN should do the binding based on those managed DLLs are used. Personally, I like the first mental model at work. But from backward compatibility point of view, I support the second mental model, because that is the unmanaged C++ vtable should look like, and by that time, there is no unknown there. I think the ball should roll back to the CLR team. Or someone should redefine what “virtual” means in .net framework instead of adopting the “virtual” from C++ if the problem is too large to be addressed there.Anonymous
March 30, 2010
Exciting reading as always in your excellent blog Eric. I fully understand that this topic is not a clear-cut question with a "everyone is happy - no problems whatsoever"-answer. I thought that I would look at a another real life scenario. (Apart from the issues we have with this in a product in our company.) I looked at the service pack changes made in one of the WPF-assemblies by Microsoft. I compared the 3.0.6920.0 version (the version you get if you installed the original 3.5-framework) of PresentationFramework.dll with the version I get if I update it using Windows Update (3.0.6920.4902). There were some methods and properties that have new overrides in the new service pack version. Some of the methods were: System.Windows.Controls.TreeViewItem: MeasureOverride System.Windows.Controls.VirtualizingStackPanel: OnGotKeyboardFocus and OnLostKeyboardFocus System.Windows.Documents.Table: BeginInit and EndInit I haven't checked what code was added in the functions mentioned, but I suspect that at least some bad things will happen if it is not executed by derived classes that was compiled with a version earlier than 3.5SP1. Keeping binary compatibility is not easy. You constantly have to make decisions what changes can be made, and what impact will they have. Like some of you have mentioned before me, there are some things you can change and some things you can't, and still have it binary compatible. But not being able to add new overrides in classes and be somewhat confident that the base-code will be executed (by the standard base-call), severely limits the things you can change. Apparently at least the developers of the WPF seems to think that, since they have created new overrides on some of the classes in a service pack. Or maybe those changes were necessary, and they estimated that not that much derived code would break by releasing the service packs. (My guess is they didn't think they were introducing problems just by overriding those methods.) Of course, this doesn't mean that this behavior has to change. It's just one more example of what some (and I go out on a limb and say, most) developers expect from the "base-call". The solution of course is, when in doubt, recompile. But that equates to: "Everytime Microsoft releases a new hotfix or service pack, recompile and update all installations of the application." That is not a very preferable scenario. Disclaimer: I compared the 3.0.6920.0-version from a Vista-machine with the 3.0.6920.4902-version on my Windows 7 machine. Since I haven't looked at the actual code added in the SP, all the changes they made, can possibly be of the type that doesn't have to be executed. Or they expected the changes to be breaking changes. My analysis of the assembly may also be faulty. Then, my apologies to the WPF-team, for thinking that you didn't fully understand all the details of how the compiler/framework worked.Anonymous
March 30, 2010
The comment has been removedAnonymous
March 30, 2010
The comment has been removedAnonymous
March 30, 2010
I'm sorry to say that the C# team is wrong wrong wrong on this one. I've never run into this issue and I don't think too many have since it hasn't cropped up all that much but nonetheless, its IMHO a pretty big problem that should be fixed ASAP. I'm not really go into any technicalities but my reasoning as to why the current behaviour is so wrong goes as follows: Imagine the original Bravo.dll implementation were something like this: class Bravo: Alpha { protected override bool DoM() { ... return whatever } } Charlie implementation is: class Charlie: Bravo { protected override M() { if (base.DoM()) base.M(); } } When I see this code I understand (at least up to now) base as the immediately less derived class in the inheritance chain. In this case that would be Bravo. I don't really care if the implementation of the methods I'm calling is in Bravo or in Alpha, i'm still calling THROUGH Bravo because thats what intuitively base means to me. If suddenly we change the Bravo implementation and add an override of M() what happens with the code in Charlie? Well, we have the really bad situation of having base mean two entirely different things in the same declaration body. That is base.DoM() really means base.DoM() but base.M() actually means base.base.M(). I'm sorry but that is plain wrong and it goes against all that Eric has written in previous posts about the design philosophy of C#.Anonymous
March 30, 2010
The comment has been removedAnonymous
March 30, 2010
The comment has been removedAnonymous
March 30, 2010
The comment has been removedAnonymous
March 30, 2010
@Bill Sheldon I dont agree as the current state of affairs does in fact let you swap as you say with no problems whatsoever. Another issue is if this should be done without recompiling Charlie. If the original implementation of Bravo.dll did have an override of M and I decided to change the implementation of M later on and issue a new version of Bravo, Charlie would consume the new version fine. Its the fact that sometimes Charlie WONT notice the changes done in Bravo what is unexpected and IMHO not right. These seemingly equivalent implementations of Bravo turn out to be quite different and that is disconcerting to say the least. class Bravo {} class Bravo {protected override void M() {}}Anonymous
March 30, 2010
This has been stated before, I just want to emphasize this once more. How can we be sure that UI libraries built on top WindowsForms or WebForms or WPF (ones with deep inheritance hierarchy) compiled under .net 3.5 still be running normally in .net 4? It's quite possible that in some control new override apperared in 4.0, and developer who put it there suggested that this is backwards compatible change, that do not need documentation.Anonymous
March 30, 2010
Some commenters think that you should always recompile all applications whenever you swap in a new dll, you seem to be missing the point. The point is that you can swap in a new library dll and most people thought that there were only two possibilities when you did that:
- The library contained no breaking changes and the application would still work, running the code in the new version of the library or
- The library did contain breaking changes, there would be a runtime error and you would be forced to recompile and perhaps change some code that depended on the library. Now we see that there is a third possibility, namely that that the library can contain changes that don't cause a runtime error but will silently fail in running the new code. If adding new method overrides is a breaking change, then it should cause a runtime error swapping in dll's containing new method overrides. This would probably break a great number of applications all over the world... If adding new method overrides is not a breaking change then those new overrides should not be bypassed and the base call should work the way most intuitively think it does. I cannot see any way this can break existing applications, the only real counter-argument is that this means work for the compiler team (and maybe for the CLR team too) and since they don't have unlimited time they have to prioritize and there may be other things that take precedence.
Anonymous
March 30, 2010
We shouldn't forget that object declares virtual methods as well. public class DerivedClass: BaseClass { private readonly int _i; public Gamma(int i) { _i = i; } public override bool Equals(object obj) { return _i == ((DerivedClass)obj)._i && base.Equals(obj); } public override int GetHashCode() { return base.GetHashCode() ^ _i; } } What if BaseClass changes, and overrides Equals? DerivedClass would ignore this change, and call object.Equals() instead. This is not what I expected at all.Anonymous
March 30, 2010
> I wouldn't expect this to change. To me in addition to the logic above, this seems like an intuitive part of static or pre-compiled code versus a dynamic language. Well, doesn't DLL mean "dynamic link library"? :-) Maybe it's time to consider a different file extension for assemblies.Anonymous
March 31, 2010
The comment has been removedAnonymous
March 31, 2010
np: the fundamental question here , I think, is whether the user IS changing the signature or not. I don't think they are--but that's because I had assumed that every method that I didn't implement really did have that little magical implicit "send it on up the chain": class Bravo2 : Alpha { public override void M() { base.M() } } This is how a LOT of things work, I think. Routed events will route through everything in the middle, whether they declare a handler or not, and if I add a new event handler half-way up the chain on the fly, my code accomodates it. So, +1 for the customer.Anonymous
March 31, 2010
I fully agree with the customer. The above arguments are not convincing. I also agree with Sandro's comment. Why would you allow virtual resolution to a private method? Is there any practical or even technical value for this? As you said, "private methods are invisible implementation details; they do not have any effect on the surrounding code that cannot see them". Fix this at the CLR level and then you are free to generate the appropriate code that nearly all of us are expecting.Anonymous
March 31, 2010
The comment has been removedAnonymous
April 01, 2010
The comment has been removedAnonymous
April 01, 2010
I just can't believe that the CLR designers really expected us to recompile our apps every time a DLL is revised. Imagine that you're a vendor with a graphical control library that depends on WPF. Every time a new service pack is released for WPF, you need to recompile your library, right? Only it doesn't matter that you have to recompile because you need to get your new DLLs to your clients. But your clients aren't the end-user; the clients of your custom control library are app authors. This means that your clients' apps will all start misbehaving when their customers download a service pack. So every time there's a new revision of WPF, you have to compile a new set of DLLs, distribute them to your clients (assuming you even know who they are), and then they have to distribute the new DLLs to their customers (assuming they even know who they are). But wait, there's more! Your clients don't know which revision of WPF will be on their customers' machines so they have to ship an install package with a different set of DLLs for each possible WPF revision. That way the right set can be installed at setup time, and the correct version can be swapped in if the app detects that the user has installed a WPF service pack. How is this different from any other change in a service pack that breaks your DLL? Almost any other kind of breaking change is something you can detect by looking at the version and adjusting your behavior accordingly. Changing a base class's overrides, though, requires a separate version for each revision of the base class DLL because while each revision could add overrides, it could also remove them. This means that there is no single version of your DLL that can work with all revisions of the base classes. I don't know about you, but I would prefer a system that didn't require what I just described.Anonymous
April 01, 2010
I would also like to put in a WOW! I imagine that the debate within Microsoft was even more passionate than the responses in these posts. Especially since, "The team all agreed that the desirable behaviour was to always dynamically bind to the closest base class" It had to be very frustrating for the C# team to not be able to create the functionality that they thought was best, and then to actually have to make the situation worse for their users between version 1.0 and 2.0 so that the C# compiler and other tools would be simpler and faster. I bet this was a bitter pill to swallow for some of the team. Thanks for sharing some of these intimate details of the decision process of the creation and evolution of C#.Anonymous
April 01, 2010
The comment has been removedAnonymous
April 01, 2010
The comment has been removedAnonymous
April 01, 2010
The comment has been removedAnonymous
April 01, 2010
oh, I think the clue is the title: - Putting a base in the middle. =)Anonymous
April 01, 2010
@Michael, Not sure why this is so difficult, but here goes:
- You distribute a .NET 3.5 application to your customers.
- Customers install .NET 3.5 SP1.
- Hosed
Anonymous
April 01, 2010
genious: So you are placing/voting yourself in category 1? What I found as of yet, is that when you go .NET4 with code contracts and then have to revert to .NET3.5 in panic, msbuild.exe gets very upset and refuses to build. Just a few lines in the .csproj-xml that makes the build confused. And so I have to manually compile and deploy .dll via ftp until when .NET4.0 sails. Hence I wrote I wanna be voting 2, but am factually in cat 1. Well not really, as this is just a few lines in xml that could be done with. I'll stick around doing it manual until 12 April. Would you still be 'Hosed' if you recompiled, sift through all warnings and set a 'supported to 3.5 sp1' that would make windows update kick in, assuming we are talking windows OS? I am not nagging, but asking.Anonymous
April 01, 2010
The comment has been removedAnonymous
April 02, 2010
The comment has been removedAnonymous
April 02, 2010
Eric, thank you for your response and your honesty – I appreciate your passion to defend yourself and your team. I am sorry that I implied that you or your team are disingenuous in anyway. I should have wrote: “It is my opinion you are fooling yourself if you think the two bugs can be compared equally.” But I wanted to expand my vocabulary after your use of “rather histrionic comments.” (I had to look up the definition) Thanks again for this blog and the great insights you provide on a weekly basis. Please continue to provide the unique looks into the C# design team’s decision process. I do know that your decisions are made with us in mind and that a lot of decisions are difficult compromises.Anonymous
April 02, 2010
The comment has been removedAnonymous
April 02, 2010
The comment has been removedAnonymous
April 02, 2010
The comment has been removedAnonymous
April 04, 2010
The comment has been removedAnonymous
April 05, 2010
I agree with what GRico said somewhere above. base can actually have two very different meanings in the same declaration space. It might be intended that way but it doesnt feel right at all. protected override Whatever() { if (base.whatever()) base.whateverElse(); } if whatever() is overriden in the "base" class understanding base class as the preceding class in the inheritance hierarchy and whateverElse() isnt, then the base keyword has unexpectedly two alltogether different meanings. I wasn't aware of this behavior at all. To me base.X intuitively was a virtual call, so all this is new. I still think that C# made a mistake here as my intuition seems to coincide with the majority of posters and probably with the majority of coders who aren't even aware of this issue at all. I'd like to point out that this is completely independant of the "dll swap" problem this post talks about and if you should recompile or not. That issue only makes the dual meaning of the base keyword visible which is my gripe with the language.Anonymous
April 15, 2010
The comment has been removed