43

Say I have this simple method:

public IEnumerable<uint> GetNumbers()
{
    uint n = 0;
    while(n < 100)
        yield return n++;
}

How would you make this thread safe? And by that I mean that you would get that enumerator once, and have multiple threads handle all the numbers without anyone getting duplicates.

I suppose a lock needs to be used somewhere, but where must that lock be for an iterator block to be thread safe? What, in general, do you need to remember if you want a thread safe IEnumerable<T>? Or rather I guess it would be a thread safe IEnumerator<T>...?

Svish
  • 152,914
  • 173
  • 462
  • 620
  • 4
    (Just in case you're not checking my comments) I've just blogged about this: http://msmvps.com/blogs/jon_skeet/archive/2009/10/23/iterating-atomically.aspx – Jon Skeet Oct 23 '09 at 21:22
  • 3
    Oh, thanks! This site should have sort of Facebook'ish notification when someone comments after your comment or something like that... hehe. – Svish Oct 24 '09 at 11:07
  • I assume that you need a thread-save Enumerator, so you should probably implement that one. – Stefan Steinegger Oct 22 '09 at 08:33

5 Answers5

45

There's an inherent problem in doing so, because IEnumerator<T> has both MoveNext() and Current. You really want a single call such as:

bool TryMoveNext(out T value)

at that point you can atomically move to the next element and get a value. Implementing that and still being able to use yield could be tricky... I'll have a think about it though. I think you'd need to wrap the "non-threadsafe" iterator in a thread-safe one which atomically performed MoveNext() and Current to implement the interface shown above. I don't know how you'd then wrap this interface back into IEnumerator<T> so that you could use it in foreach though...

If you're using .NET 4.0, Parallel Extensions may be able to help you - you'd need to explain more about what you're trying to do though.

This is an interesting topic - I may have to blog about it...

EDIT: I've now blogged about it with two approaches.

Jon Skeet
  • 1,421,763
  • 867
  • 9,128
  • 9,194
  • 4
    You should call that `TryMoveNext` to match similar items in the framework. :) – Sam Harwell Oct 22 '09 at 08:36
  • One way I can think of that would allow you to use it in foreach would be to have a class that implements IEnumerable, and internally stores the current item in a threadlocal field, this way, when a thread has successfully called MoveNext, its value is stored for it to later use, separate from other threads. – Lasse V. Karlsen Oct 22 '09 at 08:52
  • So maybe acquire lock, get an object store it in a local variable, and release lock and then yield it? A blog post about this would be nice! – Svish Oct 22 '09 at 09:08
  • 2
    @Svish: Blogged about it just now: http://msmvps.com/blogs/jon_skeet/archive/2009/10/23/iterating-atomically.aspx – Jon Skeet Oct 23 '09 at 21:21
  • Jon Skeet, the links are wrong on your answer, for shame! – AlexC Feb 19 '15 at 21:21
  • @JonSkeet Curious... How would you write tests asserting that its thread safe? I have been looking specifically for BCL code examples where Microsoft tests these guarantees and I can't find any. See for example the guarantee that ConcurrentQueue.GetEnumerator() is thread-safe. https://github.com/dotnet/coreclr/search?q=ConcurrentQueue&unscoped_q=ConcurrentQueue – John Zabroski Mar 07 '19 at 23:39
  • It's especially interesting (to me) when you consider System.Threading.Tasks implements a producer/consumer queue using ConcurrentQueue for the basic logic. – John Zabroski Mar 07 '19 at 23:45
  • @JohnZabroski: It's certainly hard to write tests that *prove* thread safety. You can write stress tests to at least give a certain amount of confidence, but that's as close as I'd normally get. – Jon Skeet Mar 08 '19 at 06:46
  • @JonSkeet I'm not asking if it's possible to try. I'm asking how you, Jon Skeet of Chuck Norris joke fame and StackOverflow 1m karma, would try to validate it. On one hand, I can see the validity in not bothering. On the other, simply exchanging ideas on how to best write such tests makes us better engineers. As for proof, tests never prove the absence of bugs, so the dilineation between a stress test and a concurrency test isn't clear to me. – John Zabroski Mar 08 '19 at 12:10
2

I just tested this bit of code:

static IEnumerable<int> getNums()
{
    Console.WriteLine("IENUM - ENTER");

    for (int i = 0; i < 10; i++)
    {
        Console.WriteLine(i);
        yield return i;
    }

    Console.WriteLine("IENUM - EXIT");
}

static IEnumerable<int> getNums2()
{
    try
    {
        Console.WriteLine("IENUM - ENTER");

        for (int i = 0; i < 10; i++)
        {
            Console.WriteLine(i);
            yield return i;
        }
    }
    finally
    {
        Console.WriteLine("IENUM - EXIT");
    }
}

getNums2() always calls the finally part of the code. If you want your IEnumerable to be thread safe, add whatever thread locks you want instead of writelines, wither using ReaderWriterSlimLock, Semaphore, Monitor, etc.

Dr. K
  • 21
  • 1
0

Well, i'm not sure, but maybe with some locks in the caller ?

Draft:

Monitor.Enter(syncRoot);
foreach (var item in enumerable)
{
  Monitor.Exit(syncRoot);
  //Do something with item
  Monitor.Enter(syncRoot);
}
Monitor.Exit(syncRoot);
Guillaume
  • 12,824
  • 3
  • 40
  • 48
0

I was thinking that you can't make the yield keyword thread-safe, unless you make it depend on an already thread-safe source of values:

public interface IThreadSafeEnumerator<T>
{
    void Reset();
    bool TryMoveNext(out T value);
}

public class ThreadSafeUIntEnumerator : IThreadSafeEnumerator<uint>, IEnumerable<uint>
{
    readonly object sync = new object();

    uint n;

    #region IThreadSafeEnumerator<uint> Members
    public void Reset()
    {
        lock (sync)
        {
            n = 0;
        }
    }

    public bool TryMoveNext(out uint value)
    {
        bool success = false;

        lock (sync)
        {
            if (n < 100)
            {
                value = n++;
                success = true;
            }
            else
            {
                value = uint.MaxValue;
            }
        }

        return success;
    }
    #endregion
    #region IEnumerable<uint> Members
    public IEnumerator<uint> GetEnumerator()
    {
        //Reset(); // depends on what behaviour you want
        uint value;
        while (TryMoveNext(out value))
        {
            yield return value;
        }
    }
    #endregion
    #region IEnumerable Members
    System.Collections.IEnumerator System.Collections.IEnumerable.GetEnumerator()
    {
        //Reset(); // depends on what behaviour you want
        uint value;
        while (TryMoveNext(out value))
        {
            yield return value;
        }
    }
    #endregion
}

You will have to decide whether each typical initiation of an enumerator should reset the sequence, or if the client code must do that.

Cecil Has a Name
  • 4,962
  • 1
  • 29
  • 31
-2

You could just return a complete sequence each time rather than use yield:

return Enumerable.Range(0, 100).Cast<uint>().ToArray();

Lee
  • 142,018
  • 20
  • 234
  • 287
  • And I also do not want to have to put the whole range into an array or anything like that. – Svish Oct 22 '09 at 09:09