yield returns within lock statement
Asked Answered
S

2

33

if i have a yield return in a lock statement does the lock get taken out on each yield (5 times in the example below) or only once for all the items in the list?

Thanks

    private List<string> _data = new List<string>(){"1","2","3","4","5"};
    private object _locker =new object();
    public IEnumerable<string> GetData()
    {
        lock (_locker)
        {
            foreach (string s in _data)
            {
                yield return s;
            }
        }
    }
Schmeltzer answered 17/5, 2010 at 8:30 Comment(0)
F
40

UPDATE 2022-Oct
Reading this answer after all these years, I felt the original tone was too harsh, and I came off as an a-hole. So I soften the tone a bit.

Sorry to resurrect this from the dead, but reading the accepted answer by Daniel, and then testing it myself I though that at least those original 10 people who up-voted should be informed of the incorrect answer. Daniel has reviewed this afterwards and updated his answer to point here.

The correct answer is: The lock is NEVER released between each yeald return.
It will only be released when the enumerator is done, i.e. when the foreach loop ends.

Where Daniel's made a left turn was by scaffolding the test incorrectly. His code is not multi-threaded, and it would always compute the same way. The lock in that code is taken only once, and since it's the same thread, it's always the same lock.

I took @Daniel's code from his answer, and changed it to work with 2 threads, one for List1 and another thread created for each iteration of List2. NOTE: This is NOT how this code should be structured, for cleaner, easier to read code, see the community wiki provided by @EZI. However, this does provide a direct comparison to Daniel's code and it fleshes out the issue with the original code.

As you can see once t2 thread is started, the threads would dead-lock, since t2 is waiting on a lock that would never be released.

The Code:

void Main()
{
    object locker = new object();
    IEnumerable<string> myList0 = new DataGetter().GetData(locker, "List 0");
    IEnumerable<string> myList1 = new DataGetter().GetData(locker, "List 1");
    IEnumerable<string> myList2 = new DataGetter().GetData(locker, "List 2");

    Console.WriteLine("start Getdata");
    // Demonstrate that breaking out of a foreach loop releasees the lock
    var t0 = new Thread(() => {
        foreach( var s0 in myList0 )
        {
            Console.WriteLine("List 0 {0}", s0);
            if( s0 == "2" ) break;
        }
    });
    Console.WriteLine("start t0");
    t0.Start();
    t0.Join(); // Acts as 'wait for the thread to complete'
    Console.WriteLine("end t0");
    
    // t1's foreach loop will start (meaning previous t0's lock was cleared
    var t1 = new Thread(() => {
        foreach( var s1 in myList1)
        {
            Console.WriteLine("List 1 {0}", s1);
            // Once another thread will wait on the lock while t1's foreach
            // loop is still active a dead-lock will occure.
            var t2 = new Thread(() => {
                foreach( var s2 in myList2 )
                {
                    Console.WriteLine("List 2 {0}", s2);
                }
            } );
            Console.WriteLine("start t2");            
            t2.Start();
            t2.Join();
            Console.WriteLine("end t2");            
        }
    });
    Console.WriteLine("start t1");
    t1.Start();
    t1.Join();
    Console.WriteLine("end t1");
    Console.WriteLine("end GetData");
}

void foreachAction<T>( IEnumerable<T> target, Action<T> action )
{
    foreach( var t in target )
    {
        action(t);
    }
}

public class DataGetter
{
    private List<string> _data = new List<string>() { "1", "2", "3", "4", "5" };
    
    public IEnumerable<string> GetData(object lockObj, string listName)
    {
        Console.WriteLine("{0} Starts", listName);
        lock (lockObj)
        {
            Console.WriteLine("{0} Lock Taken", listName);
            foreach (string s in _data)
            {
                yield return s;
            }
        }
        Console.WriteLine("{0} Lock Released", listName);
    }
}
Folacin answered 3/4, 2013 at 16:29 Comment(1)
+1 Good Call. Thanks for that. I will edit my answer to reflect this.Alike
C
6

@Lockszmith has a good catch (+1). I only post this since I find his code hard to read. This is a "community wiki". Feel free to update.

object lockObj = new object();

Task.Factory.StartNew((_) =>
{
    System.Diagnostics.Debug.WriteLine("Task1 started");
    var l1 = GetData(lockObj, new[] { 1, 2, 3, 4, 5, 6, 7, 8 }).ToList();
}, TaskContinuationOptions.LongRunning);

Task.Factory.StartNew((_) =>
{
    System.Diagnostics.Debug.WriteLine("Task2 started");
    var l2 = GetData(lockObj, new[] { 10, 20, 30, 40, 50, 60, 70, 80 }).ToList();
}, TaskContinuationOptions.LongRunning);

public IEnumerable<T> GetData<T>(object lockObj, IEnumerable<T> list)
{
    lock (lockObj)
    {
        foreach (T x in list)
        {
            System.Diagnostics.Debug.WriteLine("Thread " + Thread.CurrentThread.ManagedThreadId + " returned "  + x );
            Thread.Sleep(1000);
            yield return x;
        }
    }
}
Cartage answered 17/5, 2010 at 8:30 Comment(1)
I like your code more too, the reason the code below isn't as clear is because it was based on the accepted answer, and was meant to show the differences. Thanks for writing it cleaner.Folacin

© 2022 - 2024 — McMap. All rights reserved.