bsobaid
bsobaid

Reputation: 975

Is it thread-safe to increment an Int32 in a Timer callback?

I want to increment an integer that gets incremented in a timer event handler and read by the main and other worker threads i.e. one writer thread and multiple reader threads. Will it be thread-safe?

I have a timer in my application that runs every 5 seconds:

MyClock = new System.Threading.Timer(
    new TimerCallback(this.Ticker), null, Timeout.Infinite, Timeout.Infinite );

that I turn on like this:

MyClock.Change(5000, 5000);

If I increment an integer in the Ticker handler like this:

tickerCounter++;

can I then do a read-only access from main thread or worker threads of the same application? Will it be thread safe? Is there any chance of the reader to read a partial value, or causing a threading exception?

Upvotes: 1

Views: 679

Answers (4)

Theodor Zoulias
Theodor Zoulias

Reputation: 43886

The System.Threading.Timer component invokes the callback on the ThreadPool. There is no protection against overlapping invocations. In case the callback takes longer than the period, two subsequent invocations might be running on different ThreadPool threads at the same time. In that case it is possible to lose increments, in other words it's possible that the incremented Int32 value will be smaller than the total number of invocations. There is no risk of threading exceptions or torn values, but there is a risk that the correctness of your application will be compromised, which is arguably even worse.

In practice, if the timer is ticking every 5 seconds and the callback does nothing more than incrementing the integer value, the probability of overlapping is zero. To get a non-zero probability of overlapping you must either do heavy work inside the handler, or set the period to a very small value, or both. The danger zone is for periods under ~50 msec, because this is the maximum duration that the OS can suspend a thread (according to my experiments, demo). Of course relying on "safe period" estimations is not the way to write robust software. You are well advised to synchronize properly the increment of the integer, using the lock statement or the Interlocked APIs. Starting from .NET 6 you have also the option to switch to the PeriodicTimer component, which prevents overlapping invocations naturally, by the way it is used (example).

Upvotes: 0

Jon Skeet
Jon Skeet

Reputation: 1503290

Firstly: read Eric Lippert's blog post: What is this thing you call "thread-safe"? It will change how you think about thread safety, and how you ask similar questions in the future.

The reads will be atomic - you'll never see "half" an update - but you won't necessarily see the latest value. (That means that the next increment may see an "old" (lower) value to increment, of course.)

Additionally, the increment itself isn't inherently safe - if there were several threads incrementing at the same time, they could all read the same value, then increment locally, then write the new value - so the result could be an increment of 1, even if 10 threads were incrementing.

You should consider using Interlocked.Increment to perform the increment. If you also make the variable volatile, I believe you should be safe just to read it directly when you only want to read it.

Upvotes: 4

David Heffernan
David Heffernan

Reputation: 613461

Incrementing like this

tickerCounter++;

in multiple threads, without locking, is not threadsafe. You can use the Interlocked class to perform lock-free, threadsafe incrementing.

If only a single thread is modifying the value, with many threads reading the value, then tickerCounter++ is threadsafe in the sense that no thread will ever suffer a partial read. Of course there is still a race condition, but even if you use Interlocked, there would be a race.

Upvotes: 2

Tim
Tim

Reputation: 15247

Perhaps you could use Interlocked.Increment(ref Int32) to do the incrementing? That does it as an atomic operation.

Upvotes: 3

Related Questions