32

I normally know what "implicitly captured closure" means, however, today I came across the following situation:

public static void Foo (Bar bar, Action<int> a, Action<int> b, int c)
{
    bar.RegisterHandler(x => a(c)); // Implicitly captured closure: b
    bar.RegisterHandler(x => b(c)); // Implicitly captured closure: a
}

Why am I implicitly capturing the other action as well? If I comment either of the both lines, the other does not give me the warning. Anybody knows of what danger ReSharper is warning me?

Edit: ReSharper 8.0.1

D.R.
  • 20,268
  • 21
  • 102
  • 205
  • 2
    possible duplicate of [Why does Resharper tell me "implicitly captured closure: end, start"?](http://stackoverflow.com/questions/13633617/why-does-resharper-tell-me-implicitly-captured-closure-end-start) – Søren Jun 15 '14 at 06:15
  • 1
    No it is not a duplicate of 13633617, it is especially about the situation in which an unused variable is implicitly captured by a second lambda. – D.R. Jun 15 '14 at 07:55
  • You should read my answer on the mentioned question i think it is exactly what you are asking [here](http://stackoverflow.com/a/15843306/2250672) – quadroid Aug 21 '14 at 06:51

3 Answers3

34

The issue here is that when you close over a variable what happens behind the scenes is that the compiler creates a new unnamed type, gives that type an instance field for every variable that is closed over in that block, gives it a method for every anonymous method in that code block and then passes a single instance of that object around.

This means that the lifetime of the first delegate is keeping that closure object alive, and it has a reference to the object b, in addition to a, internally, and vice versa.

Now in your case, it's not an issue, as an Action isn't something that's particularly memory intensive, so keeping it alive for a bit longer isn't really a problem.

The C# team could, in theory, have ensured that in this particular case a new unnamed type could be created for each closure in the same block, but they chose not to as it makes the common case worse.

Servy
  • 202,030
  • 26
  • 332
  • 449
  • Thanks, I see. Is there anything I can do about it besides moving both lines to different types? – D.R. Sep 17 '13 at 20:38
  • 2
    @D.R. Well, as I said, in this case you can pretty much just not care. If they were expensive objects and being able to clear their memory earlier would matter then you could just not use closures for this. Create the new named types yourself, with the methods, and the instance fields for each action in each type, create instances of them in this method, etc. Closures don't actually let you do anything you couldn't do otherwise, they just remove the boilerplace code and made doing it a lot easier. – Servy Sep 17 '13 at 20:40
  • You can use ReSharper's annotations feature to add an ``[InstantHandle]`` attribute to the RegisterHandler method. This tells ReSharper that the lambdas are executed immediately, so the scope of the closure is limited, and ReSharper doesn't show the warning. – citizenmatt Sep 18 '13 at 15:34
  • 1
    @citizenmatt Based on the names and context shown it doesn't appear that they *are* executed instantly. My guess is that it is holding onto the delegates until later, possibly quite some time later. The issue here is that it's not particularly likely that the objects being closed over are expensive and would suffer greatly from having their lifetime extended. It's possible, just not likely. What's important here is that the actions, and any objects referenced within them, don't have an intended lifetime much shorter than `bar` or are not particularly memory intensive. – Servy Sep 18 '13 at 15:36
  • 1
    Good point. Should have made that clear. And of course, if that's the case, then the warning is useful and should prompt inspecting the code to see if extending lifetime here is appropriate or not. If it is, one can safely ignore the warning, or disable it locally using the correctly formatted comments - alt+enter on the warning will help generate them. – citizenmatt Sep 18 '13 at 18:54
0

I've seen this before; it has to do with the variables being held for the lifetime of the lambda, so if they are large it can create memory pressure.

Allan Elder
  • 4,052
  • 17
  • 19
  • Found the link to the original conversation http://devnet.jetbrains.com/thread/436232 – Allan Elder Sep 17 '13 at 20:34
  • 1
    Yes, but what makes it interesting is why the first lambda is holding onto a reference to `b`, and the second lambda is holding onto a reference to `a`. – Servy Sep 17 '13 at 20:35
0

And a warning, I was going mad over this:

List<List<string>> allowed = AllowedSCACSwaps;
foreach (List<string> c in allowed.Where(c => c.Contains(scac)))
{
    csc = openCycles.FirstOrDefault(icsc => (icsc.CustomerCode == customerCode) && c.Contains(icsc.SCAC));
    if (null != csc)
    {
        return csc;
    }
}

saying "implicit closure for customerCode"

string cc = customerCode.ToUpperInvariant();
string sc = scac.ToUpperInvariant();    List<List<string>> allowed = AllowedSCACSwaps;
    foreach (List<string> c in allowed.Where(c => c.Contains(sc)))
    {
        csc = openCycles.FirstOrDefault(icsc => (icsc.CustomerCode == cc) && c.Contains(icsc.SCAC));
        if (null != csc)
        {
            return csc;
        }
    }

Is fine.

Reason I was going insane?

scac and customerCode are both strings passed into the method. But even when I replaced customerCode with cc, I kept getting the same warning.

The closure was actually over scac but Resharper was misreporting it.

Roger Willcocks
  • 1,649
  • 13
  • 27
  • Feh, code doesn't support bold as well :(. warning was appearing here: allowed.Where(c **=>** c.Contains(scac))) – Roger Willcocks Mar 12 '14 at 01:27
  • That's not a misreport. The warnings for each closure list the variables that are *not actually used in that closure* but are getting captured by it anyway. So you should have got two warnings in the original code: one on the `Contains(scac)` warning about `customerCode`, and one on the `icsc.CustomerCode == customerCode` warning about `scac`. – Miral Jan 08 '15 at 07:16
  • Except the closure warning was line #3, not line #5.That doesn't make sense to me unless you are saying the body of the foreach is converted into an anonymous method call. – Roger Willcocks Jan 09 '15 at 10:34
  • Looking only at the top snippet, you should have got a warning on line #2 for "customerCode" and a warning on line #4 for "scac", because that's where the lambdas in question appear. – Miral Jan 09 '15 at 21:28