0

PricingHandler is injected into a BackgroundService. The question is do I need to add volatile to the _counter. Is Interlocked.Increment enough to make it thread safe?

internal sealed class PricingHandler : IPricingHandler
{
    private readonly ILogger<PricingHandler> _logger;
    private int _counter;

    public PricingHandler(ILogger<PricingHandler> logger)
    {
        _logger = logger;
    }

    public Task HandleAsync(CurrencyPair currencyPair)
    {
        //TODO: Implement some actual business logic
        if (ShouldPlaceOrder())
        {
            var orderId = Guid.NewGuid().ToString("N");
            _logger.LogInformation("Order with ID: {OrderId} has been placed for symbol: '{Symbol}'.", orderId, currencyPair.Symbol);
        }

        return Task.CompletedTask;
    }

    private bool ShouldPlaceOrder()
    {
        return Interlocked.Increment(ref _counter) % 10 == 0;
    }
}
nop
  • 4,711
  • 6
  • 32
  • 93
  • 1
    Does this answer your question? [When should the volatile keyword be used in C#?](https://stackoverflow.com/questions/72275/when-should-the-volatile-keyword-be-used-in-c), mainly a quote from Eric Lippert "Frankly, I discourage you from ever making a volatile field. Volatile fields are a sign that you are doing something downright crazy: you're attempting to read and write the same value on two different threads without putting a lock in place" – MindSwipe Jul 21 '22 at 05:59
  • 1
    In short, if you're already putting a lock in place (or using thread safe/ atomic operations, like `Interlocked.Increment` does) there is absolutely no need to mark your field as volatile – MindSwipe Jul 21 '22 at 06:03
  • Thank you, it does solve it. After .NET 2.0 runtime, it's not needed at all. – nop Jul 21 '22 at 06:04
  • 1
    @MindSwipe Eric Lippert's quote needs some interpretation. The source code of .NET is full of types with `volatile` fields, and Microsoft's engineers are obviously not crazy. Eric Lippert means that you shouldn't use `volatile` unless you know what you are doing. The problem is that you can't get this knowledge by reading the documentation, because the memory model guarantees [are not thoroughly described](https://github.com/dotnet/runtime/issues/67330 "Confusion regarding Volatile.Read guarantees, and example in the volatile docs") there, and the wording is utterly confusing. – Theodor Zoulias Jul 21 '22 at 11:39

0 Answers0