CA2000 error with 'using' statement. How to make this valid?

Question

The following code is giving me this code analysis error

CA2000 : Microsoft.Reliability : In method 'SessionSummary.SessionSummary_Load(object, EventArgs)', call System.IDisposable.Dispose on object 'entities' before all references to it are out of scope.

I am using a 'using' statement, so I am surprised on this:

    private void SessionSummary_Load(object sender, EventArgs e)
    {
        using (var entities = new DbEntities(Properties.Settings.Default.UserConnectionString))
        {
            entities.CommandTimeout = 7200;
            var sessions = from t in entities.TableName
                            where t.UserSession.Id == _id && t.Parent == 0
                            group t by new { t.UserSession, t.UserSession.SessionId } into sessionGroup
                            select new
                            {
                                Id = sessionGroup.Key.UserSession,                                   
                                Session = sessionGroup.Key.SessionId                                   
                            };

            summaryDataGridView.DataSource = sessions.Where(x => x.Time > 0.00);
            summaryDataGridView.Columns[4].DefaultCellStyle.Format = "N2";
            summaryDataGridView.Columns[4].DefaultCellStyle.Alignment = DataGridViewContentAlignment.MiddleRight;
        }
    }

Answer 1

You are performing a LINQ-style query over entities but are not actually enumerating over the result within the using block. This creates a closure issue, since the query information stored in sessions.Where(x => x.Time > 0.00) is being stored in summaryDataGridView.DataSource and there is thus a reference to entities still in memory after your code above exits.

The explanation for this is that LINQ methods provide deferred execution*, which means that neither session nor the value you're assigning to summaryDataGridView.DataSource will be evaluated in the code above.

To force evaluation, you should be able to do this:

summaryDataGridView.DataSource = sessions.Where(x => x.Time > 0.00).ToList();

The addition of ToList() above will actually cause your query to be executed and the results to be cached in memory. Furthermore, entities will have gone out of scope and you will have no more references to it via any closure; so I believe that should do the trick for you.

_{*Note: this was just the first link I found from a Google search. It looked pretty good, though.}

Answer 2

You've actually got a potentially premature disposal, rather than a late one, as entities being involved in the closure assigned to the datasource means it leaves the scope.

You've then got an entities out in the wild that won't be disposed in its real scope, which is what the analysis is complaining about, but that's unlikely to be a problem compared with the fact that it is disposed before it is last used.

Options:

1. Leave it as it is. If the above works then it is probably because disposal doesn't affect the necessary state for the closure to work. This is dicey. Betting on "probably" isn't a good idea, and may also change down the road. (I can think of one case where using an object after disposal makes sense, but it's obscure and not what you have here anyway).

2. Force the query to execute eagerly. Calling ToList() or ToArray() on the query will run it and create an in-memory result that is then used as the datasource. At best though this will be less efficient in both space and time. At worse it could be cripplingly so (depending on the size of results you are dealing with).

3. Make sure the control completes using its datasource before the scope is left. Then clear the datasource. Depending on the control in question and some other matters (in particular, if it has an explicit DataBind() method) it can be trivial, impossible or somewhere in between to do this.

4. Put entity into an instance variable. Implement IDisposable. In your Dispose() method, call it's Dispose() method. Do not add a finaliser for this, as you are only disposing a managed object.

5. Create an enumerable method that wraps the query (and the using) and then does a yield return on each item the query returns. Use this as the datasource.

5 seems the best bet for most cases. It has the advantage of not changing the code much while not adding the (potentially large, depending on data) overhead of number 2. Note that just calling AsEnumerable (which has almost the same effect on execution order) would not have the same effect, as the closure would still be leaving the block unexecuted.

Edit: An enumerable that wraps the query would be like:

private IEnumerable GetSessions()
{
    using (var entities = new DbEntities(Properties.Settings.Default.UserConnectionString))
    {
        entities.CommandTimeout = 7200;
        var sessions = from t in entities.TableName
                        where t.UserSession.Id == _id && t.Parent == 0
                        group t by new { t.UserSession, t.UserSession.SessionId } into sessionGroup
                        select new
                        {
                            Id = sessionGroup.Key.UserSession,                                   
                            Session = sessionGroup.Key.SessionId                                   
                        };

        foreach(var sess in sessions.Where(x => x.Time > 0.00))
          yield return sess;
    }
}

Then you would set change SessionSummary_Load to:

private void SessionSummary_Load(object sender, EventArgs e)
{
        summaryDataGridView.DataSource = GetSessions();
        summaryDataGridView.Columns[4].DefaultCellStyle.Format = "N2";
        summaryDataGridView.Columns[4].DefaultCellStyle.Alignment = DataGridViewContentAlignment.MiddleRight;
    }
}

Hopefully this will solve the problem, because entities never leaves the scope of the using.