1

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?

Theodor Zoulias
  • 34,835
  • 7
  • 69
  • 104
bsobaid
  • 955
  • 1
  • 16
  • 36

4 Answers4

4

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.

Jon Skeet
  • 1,421,763
  • 867
  • 9,128
  • 9,194
  • thx, its fine for me if the value is not the latest just as long as it does not raise an exception or provide half update. – bsobaid May 11 '12 at 19:40
  • Not following Jon's advice means that sometimes your call to `tickerCounter++;` will not increase the value of `ticketCounter`. The documentation for `Interlocked.Increment` has example code which demonstrates this problem. This strikes me as the kind of decision that will cause a lot of confusion and wasted debugging time later (either 1 month later when someone else is debugging your code or 3 months later when you are debugging your code). – Brian May 11 '12 at 20:15
  • @Brian While I certainly don't disagree with almost everything you said, there is only one thread doing the increment. All other threads are reading, thus, your first sentence is not relevant. – Marc May 11 '12 at 20:37
  • @Marc: Who says there's only one thread incrementing? It's a `System.Threading.Timer` with no synchronization object, so each increment will occur on a thread-pool thread, but it could be a *different* thread-pool thread each time. – Jon Skeet May 11 '12 at 20:41
  • @JonSkeet I concede, you're right. It _could (execute the increment simultaneously on multiple timer tick threads)_ happen. – Marc May 11 '12 at 20:45
  • @Marc: It doesn't have to be simultaneous. It would just have to be on multiple threads without *guaranteed* memory barriers ensuring that the value read by the second incrementing thread was the value that had been written by the first incrementing thread. I mean it's *probably* going to be fine, but hey :) – Jon Skeet May 11 '12 at 20:54
3

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

Tim
  • 14,999
  • 1
  • 45
  • 68
2

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.

David Heffernan
  • 601,492
  • 42
  • 1,072
  • 1,490
0

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).

Theodor Zoulias
  • 34,835
  • 7
  • 69
  • 104