5

I have a class:

public class Checker
{
    private HashSet<int> _hs = new HashSet<int>();

    public bool Check(int a)
    {
        return Volatile.Read(ref _hs).Contains(a);
    }

    public void Update(IEnumerable<int> items)
    {
        Volatile.Write(ref _hs, new HashSet<int>(items));
    }
}

Method Check is called from multiple threads quite often. Method Update is called from a single thread which monitors some source (database, http service etc.). Is this pattern of Volatile.Read / Volatile.Write usage correct?

mtkachenko
  • 5,389
  • 9
  • 38
  • 68
  • If it is the same structure and an element can be modified while another threads read it, it will be problem. You will need to keep readers off while writing. A semaphore seems like optimal solution, or if the types of the hash allow atomic behaviour that should also work for the writer. It will lock readers. – Love Tätting Feb 14 '19 at 12:28
  • @LoveTätting in the scenario presented, readers and writers *cannot* compete; the writer creates a **new** hash-set, then swaps the reference once it has finished; readers will always be getting a hash-set that is no longer being mutated, whether it was the "old" one or the "new" one – Marc Gravell Feb 14 '19 at 12:32
  • Ahh, thank you for pointing that out, I misunderstood – Love Tätting Feb 14 '19 at 12:35

1 Answers1

5

If you mean "will Check always use the most up to date version of the field", then yes, as a side-effect of volatility this will be the case - and swapping the entire reference is much cheaper than constantly synchronizing (.NET ensures you can't have a torn reference so the reference swap is guaranteed to be atomic).

Note: the thread-safety in this scenario is strictly dependent on the fact that the hash-set is not mutated after it has been created and the reference swapped, which is what happens in the code in the question.

You can get the same result more conveniently, though, by declaring the field as volatile:

public class Checker
{
    private volatile HashSet<int> _hs = new HashSet<int>();

    public bool Check(int a) => _hs.Contains(a);

    public void Update(IEnumerable<int> items) => _hs = new HashSet<int>(items);
}
Marc Gravell
  • 1,026,079
  • 266
  • 2,566
  • 2,900
  • 1
    I wanted to answer too fast once again and misinterpreted the question. Looks like you're right; though I'm not sure `Contains()` is guaranteed to be (or remain) thread-safe. Depends on the implementation of course. A quick looks using JustDecompile on .NET Framework 4.7 looks fine at a glance. – CodeCaster Feb 14 '19 at 12:29
  • 2
    @CodeCaster I will have to agree you're right in that the docs don't make any statement that reads don't mutate state; technically it *could* do some kind of deferred hashing or dynamic rebalancing; in reality: it doesn't - but yes, I concede that this is not a formal guarantee of the type – Marc Gravell Feb 14 '19 at 12:31