135

I was answering a question about the possibility of closures (legitimately) extending object-lifetimes when I ran into some extremely curious code-gen on the part of the C# compiler (4.0 if that matters).

The shortest repro I can find is the following:

  1. Create a lambda that captures a local while calling a static method of the containing type.
  2. Assign the generated delegate-reference to an instance field of the containing object.

Result: The compiler creates a closure-object that references the object that created the lambda, when it has no reason to - the 'inner' target of the delegate is a static method, and the lambda-creating-object's instance members needn't be (and aren't) touched when the delegate is executed. Effectively, the compiler is acting like the programmer has captured this without reason.

class Foo
{
    private Action _field;

    public void InstanceMethod()
    {
        var capturedVariable = Math.Pow(42, 1);

        _field = () => StaticMethod(capturedVariable);
    }

    private static void StaticMethod(double arg) { }
}

The generated code from a release build (decompiled to 'simpler' C#) looks like this:

public void InstanceMethod()
{

    <>c__DisplayClass1 CS$<>8__locals2 = new <>c__DisplayClass1();

    CS$<>8__locals2.<>4__this = this; // What's this doing here?

    CS$<>8__locals2.capturedVariable = Math.Pow(42.0, 1.0);
    this._field = new Action(CS$<>8__locals2.<InstanceMethod>b__0);
}

[CompilerGenerated]
private sealed class <>c__DisplayClass1
{
    // Fields
    public Foo <>4__this; // Never read, only written to.
    public double capturedVariable;

    // Methods
    public void <InstanceMethod>b__0()
    {
        Foo.StaticMethod(this.capturedVariable);
    }
}

Observe that <>4__this field of the closure object is populated with an object reference but is never read from (there is no reason).

So what's going on here? Does the language-specification allow for it? Is this a compiler bug / oddity or is there a good reason (that I'm clearly missing) for the closure to reference the object? This makes me anxious because this looks like a recipe for closure-happy programmers (like me) to unwittingly introduce strange memory-leaks (imagine if the delegate were used as an event-handler) into programs.

Community
  • 1
  • 1
Ani
  • 111,048
  • 26
  • 262
  • 307
  • 19
    Interesting. Looks like a bug to me. Note that if you don't assign to an instance field (e.g. if you return the value), it *doesn't* capture `this`. – Jon Skeet Dec 07 '11 at 16:59
  • @Jon: Yep, looks like 1. and 2. are both required. – Ani Dec 07 '11 at 17:00
  • Does it still capture `this` if `_field` is public? – Rawling Dec 07 '11 at 17:09
  • 15
    I can't repro this with VS11 Developer preview. Can repro in VS2010SP1. Seems it is fixed :) – leppie Dec 07 '11 at 17:14
  • @leppie: Thanks, that's great. Looking more and more like a bug, then. I tried to do some sensible googling hoping to find a Connect issue, but instead found this question back. :) – Ani Dec 07 '11 at 17:18
  • 2
    This also happens in VS2008SP1. For VS2010SP1, it happens for both 3.5 and 4.0. – leppie Dec 07 '11 at 17:28
  • I suppose the reason might be because you are calling the lambda from an instance method. This could be a way for the framework to enable access to instance members of the Foo instance in the closure. – Ivaylo Slavov Dec 07 '11 at 17:49
  • @leppie, Are you targeting 4.5 with VS11 when you can't repro? – Dean Kuga Dec 07 '11 at 19:02
  • 5
    Hmm, bug is an awfully big word to apply to this. The compiler just generates slightly inefficient code. Certainly not a leak, this garbage collects without a problem. It probably got fixed when they worked on the async implementation. – Hans Passant Dec 07 '11 at 19:27
  • 7
    @Hans, this would not garbage collect without a problem if the delegate would survive the lifetime of the object, and there is nothing preventing this from happening. – SoftMemes Dec 07 '11 at 19:30
  • I may be completely wrong here but as I gather, compiler keeps a reference to this, to preserve a reference to its parent i.e. Class Foo, and everything is done in reference to this reference and this reference can query its underlying properties, fields etc – user182630 Dec 09 '11 at 11:30
  • I came across the same issue a couple of months ago; in my case, it was creating an OOM as some Very Large Objects were being held onto when they should have been collected. If you file an issue on Connect I'll upvote it :) – thecoop Dec 09 '11 at 15:26
  • I bet it's a bug. The fact that the result depends on wether we store the lambda in a field or not doesn't make sense to me. I'd rather expect the behavior to be the opposite to the one Jon Skeet has described. It'd be interesting to see Mr. Lippert's comments - maybe we miss something? – Pavel Gatilov Dec 09 '11 at 18:57
  • @Pavel Gatilov: You can always send an email and ask him to take a look. There's a contact link [on his blog](https://blogs.msdn.com/b/ericlippert). – BoltClock Dec 13 '11 at 19:42

2 Answers2

24

That sure looks like a bug. Thanks for bringing it to my attention. I'll look into it. It is possible that it has already been found and fixed.

Eric Lippert
  • 647,829
  • 179
  • 1,238
  • 2,067
7

It seems to be a bug or unnecessary:

I run you exemple in IL lang:

.method public hidebysig 
    instance void InstanceMethod () cil managed 
{
    // Method begins at RVA 0x2074
    // Code size 63 (0x3f)
    .maxstack 4
    .locals init (
        [0] class ConsoleApplication1.Program/Foo/'<>c__DisplayClass1'   'CS$<>8__locals2'
    )

    IL_0000: newobj instance void ConsoleApplication1.Program/Foo/'<>c__DisplayClass1'::.ctor()
    IL_0005: stloc.0
    IL_0006: ldloc.0
    IL_0007: ldarg.0
    IL_0008: stfld class ConsoleApplication1.Program/Foo ConsoleApplication1.Program/Foo/'<>c__DisplayClass1'::'<>4__this' //Make ref to this
    IL_000d: nop
    IL_000e: ldloc.0
    IL_000f: ldc.r8 42
    IL_0018: ldc.r8 1
    IL_0021: call float64 [mscorlib]System.Math::Pow(float64, float64)
    IL_0026: stfld float64 ConsoleApplication1.Program/Foo/'<>c__DisplayClass1'::capturedVariable
    IL_002b: ldarg.0
    IL_002c: ldloc.0
    IL_002d: ldftn instance void ConsoleApplication1.Program/Foo/'<>c__DisplayClass1'::'<InstanceMethod>b__0'()
    IL_0033: newobj instance void [mscorlib]System.Action::.ctor(object, native int)
    IL_0038: stfld class [mscorlib]System.Action ConsoleApplication1.Program/Foo::_field
    IL_003d: nop
    IL_003e: ret
} // end of method Foo::InstanceMethod

Example 2:

class Program
{
    static void Main(string[] args)
    {
    }


    class Foo
    {
        private Action _field;

        public void InstanceMethod()
        {
            var capturedVariable = Math.Pow(42, 1);

            _field = () => Foo2.StaticMethod(capturedVariable);  //Foo2

        }

        private static void StaticMethod(double arg) { }
    }

    class Foo2
    {

        internal static void StaticMethod(double arg) { }
    }


}

in cl: (Note !! now the this reference is gone !)

public hidebysig 
        instance void InstanceMethod () cil managed 
    {
        // Method begins at RVA 0x2074
        // Code size 56 (0x38)
        .maxstack 4
        .locals init (
            [0] class ConsoleApplication1.Program/Foo/'<>c__DisplayClass1' 'CS$<>8__locals2'
        )

        IL_0000: newobj instance void ConsoleApplication1.Program/Foo/'<>c__DisplayClass1'::.ctor()
        IL_0005: stloc.0
        IL_0006: nop //No this pointer
        IL_0007: ldloc.0
        IL_0008: ldc.r8 42
        IL_0011: ldc.r8 1
        IL_001a: call float64 [mscorlib]System.Math::Pow(float64, float64)
        IL_001f: stfld float64 ConsoleApplication1.Program/Foo/'<>c__DisplayClass1'::capturedVariable
        IL_0024: ldarg.0 //No This ref
        IL_0025: ldloc.0
        IL_0026: ldftn instance void ConsoleApplication1.Program/Foo/'<>c__DisplayClass1'::'<InstanceMethod>b__0'()
        IL_002c: newobj instance void [mscorlib]System.Action::.ctor(object, native int)
        IL_0031: stfld class [mscorlib]System.Action ConsoleApplication1.Program/Foo::_field
        IL_0036: nop
        IL_0037: ret
    }

Exemple 3:

class Program
{
    static void Main(string[] args)
    {
    }

    static void Test(double arg)
    {

    }

    class Foo
    {
        private Action _field;

        public void InstanceMethod()
        {
            var capturedVariable = Math.Pow(42, 1);

            _field = () => Test(capturedVariable);  

        }

        private static void StaticMethod(double arg) { }
    }


}

in IL: (This pointer is back)

IL_0006: ldloc.0
IL_0007: ldarg.0
IL_0008: stfld class ConsoleApplication1.Program/Foo ConsoleApplication1.Program/Foo/'<>c__DisplayClass1'::'<>4__this' //Back again.

And in all three cases the method-b__0() - look the same:

instance void '<InstanceMethod>b__0' () cil managed 
    {
        // Method begins at RVA 0x2066
        // Code size 13 (0xd)
        .maxstack 8

        IL_0000: ldarg.0
        IL_0001: ldfld float64 ConsoleApplication1.Program/Foo/'<>c__DisplayClass1'::capturedVariable
                   IL_0006: call void ConsoleApplication1.Program/Foo::StaticMethod(float64) //Your example
                    IL_0006: call void ConsoleApplication1.Program/Foo2::StaticMethod(float64)//Example 2
        IL_0006: call void ConsoleApplication1.Program::Test(float64) //Example 3
        IL_000b: nop
        IL_000c: ret
    }

And in all 3 cases there is an reference to an static method, so it makes it more odd. So after this litle analys, i will say its an bug / for no good. !

BoltClock
  • 700,868
  • 160
  • 1,392
  • 1,356
Niklas
  • 1,753
  • 4
  • 16
  • 35
  • I suppose this means it is a BAD idea to use static methods from a parent class inside a lambda expression generated by the nested class? I just wonder if `Foo.InstanceMethod` is made static, would this remove the reference also? I'd be thankful to know. – Ivaylo Slavov Dec 14 '11 at 10:22
  • 1
    @Ivaylo: If `Foo.InstanceMethod` were also static, there would no instance in sight, and therefore no way for any sort of `this` to be captured by the closure. – Ani Dec 14 '11 at 10:54
  • 1
    @Ivaylo Slavov If the instance method was static, then the field has to be static, i did try - and there will not be an 'this pointer'. – Niklas Dec 14 '11 at 13:44
  • @Niklas, Thank you. In conclusion I suppose static methods for creating lambdas will guarantee the lack of this needless pointer. – Ivaylo Slavov Dec 15 '11 at 09:02
  • @Ivaylo Slavov, Guess so.. :) – Niklas Dec 16 '11 at 11:12