Synchronizing on an Integer Value

Synchronizing on an Integer value

You really don't want to synchronize on an Integer, since you don't have control over what instances are the same and what instances are different. Java just doesn't provide such a facility (unless you're using Integers in a small range) that is dependable across different JVMs. If you really must synchronize on an Integer, then you need to keep a Map or Set of Integer so you can guarantee that you're getting the exact instance you want.

Better would be to create a new object, perhaps stored in a HashMap that is keyed by the Integer, to synchronize on. Something like this:

public Page getPage(Integer id) {
  Page p = cache.get(id);
  if (p == null) {
    synchronized (getCacheSyncObject(id)) {
      p = getFromDataBase(id);
      cache.store(p);
    }
  }
}

private ConcurrentMap<Integer, Integer> locks = new ConcurrentHashMap<Integer, Integer>();

private Object getCacheSyncObject(final Integer id) {
  locks.putIfAbsent(id, id);
  return locks.get(id);
}

To explain this code, it uses ConcurrentMap, which allows use of putIfAbsent. You could do this:

  locks.putIfAbsent(id, new Object());

but then you incur the (small) cost of creating an Object for each access. To avoid that, I just save the Integer itself in the Map. What does this achieve? Why is this any different from just using the Integer itself?

When you do a get() from a Map, the keys are compared with equals() (or at least the method used is the equivalent of using equals()). Two different Integer instances of the same value will be equal to each other. Thus, you can pass any number of different Integer instances of "new Integer(5)" as the parameter to getCacheSyncObject and you will always get back only the very first instance that was passed in that contained that value.

There are reasons why you may not want to synchronize on Integer ... you can get into deadlocks if multiple threads are synchronizing on Integer objects and are thus unwittingly using the same locks when they want to use different locks. You can fix this risk by using the

  locks.putIfAbsent(id, new Object());

version and thus incurring a (very) small cost to each access to the cache. Doing this, you guarantee that this class will be doing its synchronization on an object that no other class will be synchronizing on. Always a Good Thing.

Thread Synchronization on Integer instance variable

TL;DR - it's because of Integer pooling. Make ab an Object (i.e. Object ab = new Object()) to guarantee that each instance of Check's locking doesn't interfere with the others.

---

I was initially puzzled as well. Interestingly enough, if you change

private Integer ab = 2;

to

private Object ab = new Object();

the synchronization goes away (you get different outputs on every run). Back with ab as an Integer, I ran your code in debug mode (with a breakpoint on the print thread name line) and found the following. Here's the first thread:

And here's the second thread.

Notice that it's actually the same object, Integer@443. Even though you thought you were getting two different objects, both ab fields in the two Check instances refer to the same object in memory! Therefore yes, there is correct synchronization. The question, then, is how saying Integer ab = 2 twice gets you the same Integer object in memory.

---

By saying Integer ab = 2, you are using autoboxing, by which a primitive value (of type int) is automatically converted into the corresponding Object type Integer. This is equivalent to calling the autoboxing method call:

private Integer ab = Integer.valueOf(2);

If we look into Integer.valueOf, we notice that it has a pool for values within a certain range:

public static Integer valueOf(int i) {
    if (i >= IntegerCache.low && i <= IntegerCache.high)
        return IntegerCache.cache[i + (-IntegerCache.low)];
    return new Integer(i);
}

For most conventional settings, this will include the value 2. Thus you are getting the same Integer value in memory when you call this method.

synchronized block for an Integer object

syncObject is changing each time you ++ it (the ++ is converting it to a primitive int, incrementing it, and then autoboxing it back to the Integer object. Integer objects are immutable ... once they are created, they cannot change.

Bottom ine is that you are not using the same syncPObj in all the threads, different threads use different syncObjects at different times to sync on.

use one object as the synchronization (call it syncObj), and declare it as a final Object:

private static final Object syncObject = new Object(); 

Then your counter should be a primitive (int) for perofrmance, call it 'counter' or something.

Synchronize on syncObject, and increment counter.

Edit: as per @jsn, the done flag is also broken in that your code has a 'tight loop' on the isAllDone() method, and that is bad practice. You should use thread[i].join() to wait (blocking) on each thread's completion, and then check the status from that. Using an ExecutorService is the 'right way'.

synchronized increment an int value

You synchronize on a mutable variable i. This variable changes its value each time, therefore each time you acquire a lock on another object. Each thread thus acquires a non-contended lock and can proceed simultaneously, as if no synchronization was in place.

Lesson: use a dedicated private static final Object lock = new Object() as a lock.

Synchronizing on an Integer value

You really don't want to synchronize on an Integer, since you don't have control over what instances are the same and what instances are different. Java just doesn't provide such a facility (unless you're using Integers in a small range) that is dependable across different JVMs. If you really must synchronize on an Integer, then you need to keep a Map or Set of Integer so you can guarantee that you're getting the exact instance you want.

Better would be to create a new object, perhaps stored in a HashMap that is keyed by the Integer, to synchronize on. Something like this:

public Page getPage(Integer id) {
  Page p = cache.get(id);
  if (p == null) {
    synchronized (getCacheSyncObject(id)) {
      p = getFromDataBase(id);
      cache.store(p);
    }
  }
}

private ConcurrentMap<Integer, Integer> locks = new ConcurrentHashMap<Integer, Integer>();

private Object getCacheSyncObject(final Integer id) {
  locks.putIfAbsent(id, id);
  return locks.get(id);
}

To explain this code, it uses ConcurrentMap, which allows use of putIfAbsent. You could do this:

  locks.putIfAbsent(id, new Object());

but then you incur the (small) cost of creating an Object for each access. To avoid that, I just save the Integer itself in the Map. What does this achieve? Why is this any different from just using the Integer itself?

When you do a get() from a Map, the keys are compared with equals() (or at least the method used is the equivalent of using equals()). Two different Integer instances of the same value will be equal to each other. Thus, you can pass any number of different Integer instances of "new Integer(5)" as the parameter to getCacheSyncObject and you will always get back only the very first instance that was passed in that contained that value.

There are reasons why you may not want to synchronize on Integer ... you can get into deadlocks if multiple threads are synchronizing on Integer objects and are thus unwittingly using the same locks when they want to use different locks. You can fix this risk by using the

  locks.putIfAbsent(id, new Object());

version and thus incurring a (very) small cost to each access to the cache. Doing this, you guarantee that this class will be doing its synchronization on an object that no other class will be synchronizing on. Always a Good Thing.

Why synchronizing on the field variable and incrementing it inside synchronized block results in print out of order?

Integers in Java are immutable. When you call ++itemCount, you're in fact performing three operations: First, the Integer is unboxed to an int with the value of Integer. Then, this primitive int is incremented, and finally the incremented int is autoboxed back to an Integer. So in fact, you eventually have different Integer instances. Since you synchronize on different instances, the synchronization is meaningless, and you see the out-of-order printing.

Synchronizing on an Integer results in NullPointerException

Well yes, that would throw a NullPointerException. Consider this pattern:

 Thread 1                     Thread 2

 putIfAbsent (puts)
 get (returns non-null)
 acquire monitor
                              putIfAbsent (doesn't put)
 remove (removes value)
                              get (returns null)
                              acquire monitor (bang!)

It's not that the "value get assigned to null" - it's that Map.get returns null if there's no entry for the given key.

It's hard to know what to recommend, as your code really doesn't do anything useful. If you can say what you're trying to achieve in your real code, we can give you better suggestions, potentially.

EDIT: As noted by Nikita, just returning the value of putIfAbsent doesn't work, as that returns the previous value, or null if it was absent - whereas you want the new value for the entry.

I suspect you'll have to synchronize access to the map, basically, to make your getLockId method atomic with respect to the remove operation.

Why does synchronized on the count itself do not work?

Because the lock isn't on the variable, it's on the object. Integer is immutable, when you increment it you get a new object, and this replaces the object you're locking on. Each time a thread tries to enter a synchronized block, it evaluates the parenthesized expression to find what it should lock on.

That creates a situation where one thread has a lock on the old object and incoming threads lock on the new object, and you can have two threads in your synchronized block.

Your fixed solution works because the threads all share the same lock and it doesn't change.

---

Related Topics