1

I am trying to count how many instances of a class generated during the run time of a process under multi-threading environment. The way how I do it is to increase a static counter in the constructor by looking at this post: How to Count Number of Instances of a Class

So in multi-threading environment, here is how i define the class:

class Television {
      private static volatile int counter = 0;
      public Television(){
             counter ++;
      }
}

However, I am not sure whether there is a potential bug with the code above since I think constructor in java does not imply synchronization and counter++ is not atomic so if two threads are creating instances simultaneously, is the code a bug somehow? but I am not quite sure yet.

Community
  • 1
  • 1
Guifan Li
  • 1,543
  • 5
  • 14
  • 28
  • 2
    Correct, there is a bug, because the read of counter and write to counter aren't atomically executed. Use `AtomicInteger`. – Andy Turner Nov 14 '16 at 21:13
  • Use an atomic counter. tutorial/essential/concurrency/atomicvars.html – Just another Java programmer Nov 14 '16 at 21:18
  • And of course; the one interesting question is: why do you want to do that? What kind of problem do you intend to solve by knowing about the number of instances? – GhostCat Nov 14 '16 at 21:20
  • @GhostCat well, it's occasionally useful (very occasionally); my question would be why make it a static property of `Television`, as opposed to, say, an instance variable of a `TelevisionFactory`. What with mutable global state being pure evil. – Andy Turner Nov 14 '16 at 21:21
  • @AndyTurner Thanks very much and it is very interesting. Could you explain how to use TelevisionFactory to count ? – Guifan Li Nov 14 '16 at 22:52
  • @GuifanLi a [factory](https://en.wikipedia.org/wiki/Factory_method_pattern) is just something that creates instances, e.g. it has a method which invokes `new Television()`. The variable can be a property of that class, rather than `Television`, so you can e.g. count the number of instances each factory has created. – Andy Turner Nov 14 '16 at 22:55
  • @AndyTurner I got that, thanks a lot. – Guifan Li Nov 14 '16 at 22:56

3 Answers3

3

There is a bug in this code (specifically, a race condition), because the read of counter and write to counter aren't atomically executed.

In other words, two threads can read the same value of counter, increment that value, and then write the same value back to the variable.

Thread 1    Thread 2
========    ========

Read 0
            Read 0
Increment
            Increment
Write 1
            Write 1

So the value would be 1, not 2, afterwards.

Use AtomicInteger and AtomicInteger.incrementAndGet() instead.

Andy Turner
  • 137,514
  • 11
  • 162
  • 243
2

As counter++ is NOT atomic, you can replace it with JDK's AtomicInteger which is threadsafe.

You can AtomicInteger's use getAndIncrement() method as shown below:

class Television {
      private static final AtomicInteger counter = new AtomicInteger();
      public Television(){
            counter.getAndIncrement();
      }
}

An AtomicInteger is used in applications such as atomically incremented counters, and cannot be used as a replacement for an Integer.

You can look here

Andy Turner
  • 137,514
  • 11
  • 162
  • 243
Vasu
  • 21,832
  • 11
  • 51
  • 67
  • No need to mark the AtomicInteger volatile. Under the hood, it uses a volatile variable to store the value. – jgitter Nov 14 '16 at 21:20
1

There are two ways here to bypass the underlying "++ on int" not being an atomic operation:

A) as others suggested, use AtomicInteger

B) introduce a common LOCK that all ctors can be using to sync on; like:

private final static Object LOCK = new Object();

public Television() {
  synchronized (LOCK) { 
    counter++; 
  }
Andy Turner
  • 137,514
  • 11
  • 162
  • 243
GhostCat
  • 137,827
  • 25
  • 176
  • 248