Synchonous Yield messing up my foreach loop!


I recently got stung while debugging a colleagues code, by my lack of understanding of the Yield keyword! We had a foreach loop which looks like this;

foreach (PurchaseOrderLine aLine in PurchaseOrderLine.LoadExtractLines(extractTime, systemId)) {
  //do some processing stuff...
  PurchaseOrder.MarkAsExported(lastId, extractTime);
}

The LoadExtractLines returns an IEnumerable and uses yield to return each line- the stored proc it runs is quite intensive- the whole thing looks like this;

public static IEnumerable<PurchaseOrderLine> LoadExtractLines(DateTime cutOff,int systemId)
        {
            using (SqlConnection conn = new SqlConnection(SingleAccess.Instance.ConnectionToUse))
            {
                conn.Open();
                using (SqlCommand cmd = new SqlCommand("Get_PurchaseOrderLinesToExtract", conn))
                {
                    cmd.CommandType = CommandType.StoredProcedure;
                    cmd.Parameters.AddWithValue("@CutOff", cutOff.ToString("dd MMM yyyy HH:mm:ss"));
                    cmd.Parameters.AddWithValue("@SystemId", systemId);
                    using (SqlDataReader reader = cmd.ExecuteReader())
                    {
                        while (reader.Read())
                        {
                            yield return LoadLine(reader);
                        }
                    }
                }
            }
        }

MarkAsExported runs quite an intensive update procedure on the database. As more and more data came into the system we started to see Sql Timeouts, and upon running a trace noticed something strange. Logging the RPC:Start and Completed events of the stored procs, the Get_PurchaseOrderLinesToExtract proc which feed’s the for loop was starting, then the update was starting before the Get_ had finished- the two were running side by side, causing the timeout’s!

Turns out the foreach loop started the moment it received it’s first row, yielded back from the LoadExtractLine method- which in retrospect does make sense! The solution was to convert the Load method to populate a local List<> then return the whole thing once complete, removing the yield statement alltoghether and forcing the process to wait for the entire result set to be complete before starting the loop.

, ,

  1. #1 by Paul on September 8, 2009 - 16:16

    Ooooops, should never of started using the bloody Yield way in the first place!

  2. #2 by shawson on September 10, 2009 - 11:24

    lol- it was nothing to do with me ‘guv- i’m just the lucky chap sent in to fix it all when it all goes tit’s up!

(will not be published)