Possible unintended reference comparison worked as intended

Question

I have code similar to the following:

this.Session[key] = "foo";

if ((this.Session[key] ?? string.Empty) == "foo")
{
    //do stuff
}

This, of course, creates a "Possible unintended reference comparison worked as intended" situation. The solution to this is well documented here, and I already knew that the fix was to cast the session variable to a string as soon as I saw the code.

However, the code is years old and has never been changed since it was originally written. Up until this week in our test environment, the if statement evaluated as true and executed the //do stuff section. This erroneous code IS STILL WORKING as intended in our production environment.

How can this be? There's no reason this code as written should have ever worked as intended; and yet it did, and still does in production. And what would have changed to make this code that should not have worked but did, suddenly stop working (or rather, behave like it always should have)?

Answer 1

The string literal "foo" is interned. It means that every time it's used, the same object is referenced.

The common language runtime conserves string storage by maintaining a table, called the intern pool, that contains a single reference to each unique literal string declared or created programmatically in your program. Consequently, an instance of a literal string with a particular value only exists once in the system.

For example, if you assign the same literal string to several variables, the runtime retrieves the same reference to the literal string from the intern pool and assigns it to each variable.

That's why object.ReferenceEquals("foo", "foo") is true.

If you do the same with a dynamically created string, it will not be interned and the references will not be equal

object str = new StringBuilder().Append("f").Append("oo").ToString(); // str = "foo"
object.ReferenceEquals(str, "foo");                                   // false

String interning can work differently depending on implementation, that's why you can get different behavior on different environments when comparing strings by reference.

Answer 2

You were lucky that the comparison worked! In your example the string "foo" is interned, i.e. both string literals are stored once and have both the same reference. However, if one of the two foo's is defined in another assembly or is de-serialized or is somehow constructed in code (e.g. string s = "f"; string t = s + "oo";), then the references might be different. Since Session[key] is typed as object a reference comparison will be performed.

The coalesce is not necessary, however a casting is:

if ((string)Session[key] == "foo") { ... } // This will perform a textual comparison.

You could also write this (since Equals is polymorphic):

if (Session[key].Equals("foo")) { ... } // This will perform a textual comparison.

It is not enough to compare 2 string values, they must statically be typed as string in order to be compared as strings.

Related, extremely interesting post of Jon Skeet on this subject: https://stackoverflow.com/a/3678810/880990