I was investigating some strange object lifetime issues, and came across this very puzzling behaviour of the C# compiler:
Consider the following test class:
class Test
{
delegate Stream CreateStream();
CreateStream TestMethod( IEnumerable<string> data )
{
string file = "dummy.txt";
var hashSet = new HashSet<string>();
var count = data.Count( s => hashSet.Add( s ) );
CreateStream createStream = () => File.OpenRead( file );
return createStream;
}
}
The compiler generates the following:
internal class Test
{
public Test()
{
base..ctor();
}
private Test.CreateStream TestMethod(IEnumerable<string> data)
{
Test.<>c__DisplayClass1_0 cDisplayClass10 = new Test.<>c__DisplayClass1_0();
cDisplayClass10.file = "dummy.txt";
cDisplayClass10.hashSet = new HashSet<string>();
Enumerable.Count<string>(data, new Func<string, bool>((object) cDisplayClass10, __methodptr(<TestMethod>b__0)));
return new Test.CreateStream((object) cDisplayClass10, __methodptr(<TestMethod>b__1));
}
private delegate Stream CreateStream();
[CompilerGenerated]
private sealed class <>c__DisplayClass1_0
{
public HashSet<string> hashSet;
public string file;
public <>c__DisplayClass1_0()
{
base..ctor();
}
internal bool <TestMethod>b__0(string s)
{
return this.hashSet.Add(s);
}
internal Stream <TestMethod>b__1()
{
return (Stream) File.OpenRead(this.file);
}
}
}
The original class contains two lambdas: s => hashSet.Add( s ) and () => File.OpenRead( file ). The first closes over local variable hashSet, the second closes over local variable file. However, the compiler generates a single closure implementation class <>c__DisplayClass1_0 that contains both hashSet and file. As a consequence, the returned CreateStream delegate contains and keeps alive a reference to the hashSet object that should have been available for GC once TestMethod returned.
In the actual scenario where I've encountered this issue, a very substantial (ie, >100mb) object is incorrectly enclosed.
My specific questions are:
- Is this a bug? If not, why is this behaviour considered desirable?
Update:
The C# 5 spec 7.15.5.1 says:
When an outer variable is referenced by an anonymous function, the outer variable is said to have been captured by the anonymous function. Ordinarily, the lifetime of a local variable is limited to execution of the block or statement with which it is associated (§5.1.7). However, the lifetime of a captured outer variable is extended at least until the delegate or expression tree created from the anonymous function becomes eligible for garbage collection.
This would appear to be open to some degree of interpretation, and does not explicitly prohibit a lambda from capturing variables that it does not reference. However, this question covers a related scenario, which @eric-lippert considered to be a bug. IMHO, I see the combined closure implementation provided by the compiler as a good optimization, but that the optimization should not be used for lambdas which the compiler can reasonably detect may have lifetime beyond the current stack frame.
- How do I code against this without abandoning the use of lambdas all together? Notably how do I code against this defensively, so that future code changes don't suddenly cause some other unchanged lambda in the same method to start enclosing something that it shouldn't?
Update:
The code example I've provided is by necessity contrived. Clearly, refactoring lambda creation out to a separate method works around the problem. My question is not intended to be about design best practices (as well covered by @peter-duniho). Rather, given the content of the TestMethod as it stands, I'd like to know if there is any way to coerce the compiler to exclude the createStream lambda from the combined closure implementation.
For the record, I'm targeting .NET 4.6 with VS 2015.