c# - Linq 2 Sql - Already open datareader Issue

Question

I am busted with a weird bug in my app. It comes in levels or error messages:

1. There is already and open datareader associated. Then comes
2. Invalid attempt to call Read when reader is closed. Then comes
3. Index was outside the bounds of the array. Then comes
4. Specified cast is not valid.

Let me explain first what's my code is doing: I am using Repository Pattern for my Linq-Sql app. From that repository I calling this method

 internal static IEnumerable<ParentChild> GetAllCategoriesAndSubcategories()
        {
            lock (Context) // lock is implemented just before asking question, to check whether it can solve the issue or not...
            {
                return from p in Context.Categories
                       let relatedchilds = (from c in Context.SubCategories
                                            where c.CategoryId == p.Id
                                            select c).Take(5)
                       select new ParentChild
                       {
                           Parent = p,
                           Childs = relatedchilds
                       };
            }

        }

This method is picking rows from two tables, Parent and Child and return the result as a new collection of class

public class ParentChild
    {
        public Category Parent { get; set; }
        public IEnumerable<SubCategory> Childs { get; set; }
    }

Sometimes it works fine but when the traffic increases and concurrency then in that case i start getting these errors. Coming to the issue, From UI i am consuming IEnumerable<ParentChild> GetAllCategoriesAndSubcategories() to display it in heirarchy.

At UI i am using this method to render the text:

 /// <summary>
        /// Write categories Jquery html to the Category usercontrol
        /// </summary>
        private void WriteCategories()
        {

            // retrieves all categories and its subcategories as a generic list of ParentChild
            var dt = CategoryRepository.GetAllCategoriesAndSubcategories();

            //Conversion of dynamic jquery html string starts here
            var sb = new StringBuilder();
            sb.AppendLine(" <div class='widget_box' id='category'>");
            sb.AppendLine("     <div class='wintitle'>");
            sb.AppendLine("         <div class='inner_wintitle'>Categories</div>");
            sb.AppendLine("     </div>");
            sb.AppendLine("     <div class='winbody'>");
            sb.AppendLine("         <ul class='categories'>");
            var i = 1;
            foreach (ParentChild item in dt) //<--* BUGGY PART*
            {
                sb.AppendLine(
                    string.Format("<li class='catetitle' id='catetitle{0}'><a href='subcategory.aspx?cid={1}&cname={2}'>{2}</a></li>", i,
                                  item.Parent.Id, item.Parent.Name));
                sb.AppendLine(
                    string.Format("<li style='display:none;' class='category_sub' id='subcategory{0}' ><div><ul>", i));
                foreach (var subCategory in item.Childs)
                {
                    sb.AppendLine(string.Format("<li><a href='subcategory.aspx?cid={0}&cname={1}&scid={2}&scname={3}'>{3}</a></li>", item.Parent.Id,
                                                item.Parent.Name, subCategory.Id, subCategory.Name));
                }
                sb.AppendLine(
                    string.Format(
                        "<li class='catetitle' id='catetitle{0}'><a href='subcategory.aspx?cid={1}&cname={2}'>View all categories</a></li>",
                        i, item.Parent.Id, item.Parent.Name));
                sb.AppendLine("</ul></div></li>");
                i++;
            }
            sb.AppendLine("</div></ul></div>");
            //Conversion of dynamic jquery html string ends here

            // javascript function to display the subcategories when mouse is hovered to the category
            sb.AppendLine("<script type='text/javascript'>init_categories();</script>");
            ucCategories1.CategoryHtml = sb.ToString(); // Generated text is finally set to the usercontrols property.
        }

I am getting the bug @ foreach (ParentChild item in dt). Please help me.

Suggestion Required: I am using this as my repo pattern implementation:

 internal sealed class LeadsRepository : IRepository<BuySell>
    {
        private static readonly BusinessBazaarDataContext Context;

        static LeadsRepository()
        {
            Context = new BusinessBazaarDataContext();
        }
}

I don't thinks its a good way to use Datacontext. Please suggest me... Thanks

Answer 1

Switch to List<ParentChild> and return a fully populated List from your GetAllCategoriesAndSubcategories. Your lock would then be doing what you intende (so I assume).

Specifically, you need to do:

internal static IList<ParentChild> GetAllCategoriesAndSubcategories()
    {
        lock (Context) // lock is implemented just before asking question, to check whether it can solve the issue or not...
        {
            return (from p in Context.Categories
                   let relatedchilds = (from c in Context.SubCategories
                                        where c.CategoryId == p.Id
                                        select c).Take(5)
                   select new ParentChild
                   {
                       Parent = p,
                       Childs = relatedchilds
                   }).ToList();
        }

    }

Answer 2

There are two issues in your code.

The immediate cause of the bug is the one Brett Veenstra identifes. Without the ToList() call, you return a query (of type IQueryable). The query will not be executed until it is enumerated (that is, looped through). You don't do that within your lock block, instead it is done whenever the result is used.

Brett's remedy to that is correct. Calling ToList() on the query will run the query and store the results in a list in memory within the using block. You need to add one more ToList() query though, to the realtedChilds statement. Otherwise you'll have the same problem there.

The other, larger issue is that you are sharing one data context between threads. The data context was never designed for that. Instead, the data context is meant to be a unit of work. Create one when you need it and dispose of it quickly:

using(context = new MyDataContextClass())
{
  return (from p in Context.Categories
          let relatedchilds = (from c in Context.SubCategories
                               where c.CategoryId == p.Id
                               select c).Take(5).ToList()
          select new ParentChild
          {
            Parent = p,
            Childs = relatedchilds
          }).ToList();
}

In your case where you have wrapped the data context in a repository, it might make sense to let the repository be the unit of work and have the context as a member on the repository. That means you would have your repository implement IDisposable and dispose of the member data context in the Dispose() method.

As a side note, there is yet another problem with your code. It will be executed in two steps. First it will get all the parents, then for each parent it will hit the database once more asking for its children. With more than a few 10s of parents this will be a performance issue.

The correct way to do it in linq-to-sql is to ask for all children, doing a ToList() followed by a linq-to-objects GroupBy(). I wrote a blog entry about it some time ago.

Answer 3

Yes, Agree to ToList

Reason:

"IEnumerable" performs all operations in C# code, i.e. linq-to-objects. This means all the action will happen in C#, hence it is Connected in nature

Once you calls the data by using IEnumerable, all the data will fetched from the database and sent to .net. This can degrade performance drastically (For example database indexes won't be used by linq-to-objects), but on the other hand linq-to-objects is more flexible, since it can execute arbitrary C# code instead of being limited by what your linq provider can translate to SQL.