0

NOTE The code below is from asp.net.


If I have the (poorly written) code below

AmazonS3 s3Client = Amazon.AWSClientFactory.CreateAmazonS3Client();

// ...
// details elided
// ...

BackgroundWorker worker = new BackgroundWorker();

worker.DoWork += new DoWorkEventHandler((s, args) =>
{
    s3Client.PutObject(titledRequest);
});

new Thread(() => worker.RunWorkerAsync()).Start();

Will the garbage collector be smart enough to never, ever collect the s3Client object until the background worker is done with it?


Note, I'm kicking off the background worker inside of a thread only in order to fix an annoying error that gets raised within asp.net that happens when I fire off the background worker directly.

Adam Rackis
  • 82,527
  • 56
  • 270
  • 393
  • Exactly. This is what the GC is for. Until the thread loses all references, the s3client will be kept! – quetzalcoatl Nov 19 '12 at 19:25
  • Why are you starting a new thread just to run `RunWorkerAsync()`. It's an asynchronous method to begin with; the contents of the method just create a new thread, configure it a bit, and then start it. There's no need to do that in a background thread; you're adding pointless overhead by doing so. You also don't need to define the delegate used when using lambdas, you can remove `new DoWorkEventHandler` entirely; it'll be implied based on the context. – Servy Nov 19 '12 at 19:35
  • @Servy Thanks for the tip about not needing to define the delegate. On the other point, there's this incredibly annoying error asp.net throws when you start a backgroundWorker. From searching around there didn't seem to be a simple fix, so I used this cheesy workaround. – Adam Rackis Nov 19 '12 at 19:37
  • @AdamRackis You didn't mention that this was in an ASP environment. In that respect, `BackgroundWorker` simply isn't designed to work in an ASP environment. It's designed to work in a winform/WPF type of environment. You should either be doing your long running work in a new thread directly, without using `BackgroundWorker` or using `Task` to start a new long running task. – Servy Nov 19 '12 at 19:39
  • @Servy - I did mention it in the note at the end of my question, but regardless, in an asp.net environment are you saying the GC will work differently, and in the above code, S3 client might actually get disposed? – Adam Rackis Nov 19 '12 at 19:41
  • @AdamRackis By mention it I meant tags really. No, it won't change the garbage collection at all, it's simply that the entire purpose of using a `BackgroundWorker` is to deal with interactions with the UI thread. You have no UI thread. You can't use it to run UI code when the task is completed, to report progress, etc., and that's the entire reason for using it in the first place. As I've said, you should either just start a new thread directly, or, better yet, use `Task` to execute the code in another thread. – Servy Nov 19 '12 at 19:44
  • @Servy - thanks, I didn't know that about BackgroundWorker - in looking at my code, the BW isn't doing much; a straight thread should work just fine. Thanks again. – Adam Rackis Nov 19 '12 at 19:49

2 Answers2

5

Yes, it will. The compiler will generate a new class for you that contains fields for each of the locals you reference in a closure. The closure body will be emitted into a method on that class, and all of the locals in the containing function will be rewritten by the compiler to reference fields on that closure object.

All of this magic happens at compile-time; the runtime does not need to know anything about it. Since the runtime is already smart enough not to collect an object that is the target of a delegate, the lifetimes of locals referenced by a closure are guaranteed to extend to the lifetime of the resulting delegate object.

To illustrate, the compiler is going to spit out something like this:

[System.Runtime.CompilerServices.CompilerGeneratedAttribute]
internal class ClosureImplementation // See note 1
{
    public AmazonS3 s3Client;

    public void Method(object s, EventArgs args)
    {
        s3Client.PutObject(titledRequest); // See note 2
    }
}

Then, in your method, this is emitted instead:

ClosureImplementation closure = new ClosureImplementation();
closure.s3Client = Amazon.AWSClientFactory.CreateAmazonS3Client();

// ...

worker.DoWork += closure.Method;

Notes:

  1. The name of the generated class is chosen by the compiler; ClosureImplementation is just an example.
  2. I don't have enough context to know where titledRequest comes from, so I intentionally did not address how the compiler will handle that.
cdhowie
  • 158,093
  • 24
  • 286
  • 300
  • Wow - great answer, thank you. And I loved your answer [here](http://stackoverflow.com/a/4147863/352552) btw :) – Adam Rackis Nov 19 '12 at 19:25
3

In short, yes.

You're using a lambda for your event handler, and you're closing over a local variable. That means that you're using a variable from a scope outside of the block the lambda is defined in. This means that, when the lambda is translated by the compiler into a "real" method (inside a class, with a real name and an object instance and all) it will contain a reference to the local variable that you closed over as a field of that class. This will keep that object in scope until after the lambda is no longer being referenced anywhere nor executing anywhere.

Servy
  • 202,030
  • 26
  • 332
  • 449
  • Note that this applies to anonymous delegates too, not just lambda expressions. – cdhowie Nov 19 '12 at 19:47
  • @cdhowie That's true, but I consider lambdas to render anonymous delegate obsolete. There really isn't a need for them anymore, and I see few people continuing to use them. – Servy Nov 19 '12 at 19:54
  • Lambdas don't have any syntax that means "match the expected argument types and don't use them." I still use `delegate { ... }` because there is no equivalent syntax for lambdas -- with a lambda you have to provide an argument list. Additionally, lambdas will prefer an `Expression`-based overload method when possible, and will fall back on a delegate-based overload otherwise. It may be desirable in some cases to force the usage of a delegate overload instead of an `Expression` overload, so you would use the anonymous delegate syntax in that case as well. – cdhowie Nov 19 '12 at 19:57