1
\$\begingroup\$

My system is a .NET Core 7 console app, which starts some background threads. I need to pass some int values (counters, etc.) between the main and background threads, in a thread-safe manner.

Although I can pass an int by reference (e.g. DoFoo(ref myInt)), I can't assign it in the background thread (e.g. I can't pass it into the background thread class' constructor and save it as a field). So I decided to wrap it in a reference type, which I can pass that way.

The following class works, but I'd like to know if it is truly thread-safe (have I missed something?) and whether it can be improved.

public sealed class ConcurrentInt
{
  private int _value;
  public ConcurrentInt(int value) => _value = value;

  // update value atomically
  public void Add(int value) => Interlocked.Add(ref _value, value);

  // read freshest value
  private int Get() => Interlocked.CompareExchange(ref _value, 0, 0);

  // allow `ci = ci + 1` and `ci += 1` and `ci += 1234`
  public static ConcurrentInt operator + (ConcurrentInt concurrentInt, int x)
  {
    _ = concurrentInt ?? throw new ArgumentNullException(nameof(concurrentInt));
    concurrentInt.Add(x);
    return concurrentInt;
  }

  // allow `int x = ci` instead of `int x = ci.Get()`
  public static implicit operator int(ConcurrentInt concurrentInt) {
    _ = concurrentInt ?? throw new ArgumentNullException(nameof(concurrentInt));
    return concurrentInt.Get();
  }

  // ci.ToString("N0")
  public string ToString(string? format = null) => Get().ToString(format);
}
\$\endgroup\$
8
  • \$\begingroup\$ Why do you need the implicit operator? \$\endgroup\$ Commented Nov 23, 2023 at 14:49
  • \$\begingroup\$ @PeterCsala Nicer syntax in the caller, e.g. var x = ci; instead of var x = ci.Get();. \$\endgroup\$
    – lonix
    Commented Nov 23, 2023 at 14:51
  • \$\begingroup\$ Then why do you expose the Get as well? \$\endgroup\$ Commented Nov 23, 2023 at 14:58
  • \$\begingroup\$ @PeterCsala Thanks. I guess I used both forms in the calling code - I suppose you are right, it is neater to expose just one of them. But with Add, I use this type as a readonly field in the caller, so it's convenient to use Add. \$\endgroup\$
    – lonix
    Commented Nov 23, 2023 at 15:01
  • 1
    \$\begingroup\$ @PeterCsala Ha :-)... No, I only need increment (addition), for now. For subtraction I could pass a negative into Interlocked.Add. I doubt I'll ever need multiplication or division. This is not to perform arithmetic, just to "keep track of things" - how many times something happened, was used, total counters, etc. \$\endgroup\$
    – lonix
    Commented Nov 23, 2023 at 15:47

2 Answers 2

3
\$\begingroup\$

Based on the discussion in the comments section I think the code can be greatly simplified:

public sealed class Counters
{
    private int intCounter = 0;
    public int IntCounter
    {
        get => Interlocked.CompareExchange(ref intCounter, 0, 0);
        set => Interlocked.Exchange(ref intCounter, value); 
    }
}

This allows you to hide the usage of Interlocked while exposing the data as int.

Here is a simple usage example:

var c = new Counters();
var t = new List<Task>()
{
    Task.Run(async () => { c.IntCounter += 100; await Task.Yield(); c.IntCounter++; }),
    Task.Run(async () => { ++c.IntCounter; await Task.Yield(); c.IntCounter = c.IntCounter + 20; })
};

await Task.WhenAll(t);
Console.WriteLine(c.IntCounter);

The main drawback of this design is that it allows c.IntCounter = -1 as well.

\$\endgroup\$
2
  • 1
    \$\begingroup\$ You've converted the tricky bits into a property, which is an interesting alternative. Thanks! \$\endgroup\$
    – lonix
    Commented Nov 25, 2023 at 12:40
  • 1
    \$\begingroup\$ Easy fix with: set => Interlocked.Exchange(ref intCounter, value < 0 ? 0 : value); \$\endgroup\$
    – Rick Davin
    Commented Nov 26, 2023 at 17:56
1
\$\begingroup\$
  // read freshest value
  private int Get() => Interlocked.CompareExchange(ref _value, 0, 0);

Using an Interlocked.CompareExchange for a read is relatively heavy, you could use an Interlocked.Add with 0 and return the result. Your own Add could return the new value as well for that matter, that can make it more useful, almost for free.

Consider also whether you really need the read to be interlocked, as opposed to it being merely atomic. But the consequences are subtle.

_ = concurrentInt ?? throw new ArgumentNullException(nameof(concurrentInt));

If the result was used it would be a common pattern, but with throwing away the result this is unusual. You could use ArgumentNullException.ThrowIfNull. Calling a method that throws instead of a "bare" throw also makes it more likely that uses of operator + can be inlined.

\$\endgroup\$
1
  • \$\begingroup\$ "...common pattern, but with throwing away..." that is the syntax currently used in much of MS' documents. Your take on this matter as well as the inlining trick is interesting, thanks. \$\endgroup\$
    – lonix
    Commented Nov 25, 2023 at 12:38

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.