65

I am getting this warning ("Implicity captured closure: this") from Resharper: does this mean that somehow this code is capturing the entire enclosing object?

    internal Timer Timeout = new Timer
                            {
                                Enabled = false,
                                AutoReset = false
                            };
    public Task<Response> ResponseTask
    {
        get
        {
            var tcs = new TaskCompletionSource<Response>();

            Timeout.Elapsed += (e, a) => tcs.SetException(new TimeoutException("Timeout at " + a.SignalTime));

            if (_response != null) tcs.SetResult(_response);
            else ResponseHandler += r => tcs.SetResult(_response);
            return tcs.Task;
        }
    }

I'm not sure how or why it's doing so - the only variable it should be capturing is the TaskCompletionSource, which is intentional. Is this actually a problem and how would I go about solving it if it is?

EDIT: The warning is on the first lambda (the Timeout event).

Aaron Maslen
  • 1,021
  • 1
  • 8
  • 10
  • 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:19

2 Answers2

27

It seems that the problem isn't the line I think it is.

The problem is that I have two lambdas referencing fields in the parent object: The compiler generates a class with two methods and a reference to the parent class (this).

I think this would be a problem because the reference to this could potentially stay around in the TaskCompletionSource object, preventing it from being GCed. At least that's what I've found on this issue suggests.

The generated class would look something like this (obviously names will be different and unpronounceable):

class GeneratedClass {
    Request _this;
    TaskCompletionSource tcs;

    public lambda1 (Object e, ElapsedEventArgs a) {
        tcs.SetException(new TimeoutException("Timeout at " + a.SignalTime));
    }

    public lambda2 () {
        tcs.SetResult(_this._response);
    }
}

The reason the compiler does this is probably efficiency, I suppose, as the TaskCompletionSource is used by both lambdas; but now as long as a reference to one of those lambdas is still referenced the reference to Request object is also maintained.

I'm still no closer to figuring out how to avoid this issue, though.

EDIT: I obviously didn't think through this when I was writing it. I solved the problem by changing the method like this:

    public Task<Response> TaskResponse
    {
        get
        {
            var tcs = new TaskCompletionSource<Response>();

            Timeout.Elapsed += (e, a) => tcs.SetException(new TimeoutException("Timeout at " + a.SignalTime));

            if (_response != null) tcs.SetResult(_response);
            else ResponseHandler += tcs.SetResult; //The event passes an object of type Response (derp) which is then assigned to the _response field.
            return tcs.Task;
        }
    }
Aaron Maslen
  • 1,021
  • 1
  • 8
  • 10
  • Moving the if statement or the Timeout event line to a separate method removes the warning, but I feel like it's not an ideal solution. – Aaron Maslen Oct 18 '12 at 13:52
13

It looks like _response is a field in your class.

Referencing _response from the lambda will capture this in the closure, and will read this._response when the lambda executes.

To prevent this, you can copy _response to a local variable and use that instead. Note that that will cause it to use the current value of _response rather than its eventual value.

SLaks
  • 868,454
  • 176
  • 1,908
  • 1,964
  • I should have mentioned, the warning is on the first lambda (the Timeout event) not the second. Also, what you're describing is the capture of _response, not the "this" reference – Aaron Maslen Oct 18 '12 at 13:08
  • @user1447072: Mentally replace `_response` with `Timeout` in the answer :) – Jon Oct 18 '12 at 13:12
  • It shouldn't be capturing the Timeout variable, though? (it's not referenced in the lambda anywhere). – Aaron Maslen Oct 18 '12 at 13:16
  • @AaronMaslen#1: No. Lambdas capture _variables_ or parameters, such as `this`. `_response` is really `this._response`, which captures `this`. – SLaks Oct 18 '12 at 13:30
  • From what I remember, I don't think it does capture `this` - in the sense that the compiler-generated class would only contain a single field of type `Response`, that is a reference to `this._response`. Would have to check the disassembly to confirm, though. This is a moot point, anyway, as this line isn't the line causing the warning. – Aaron Maslen Oct 18 '12 at 13:37
  • See my answer below, I think I've gotten a better grasp on the problem now – Aaron Maslen Oct 18 '12 at 13:53