Are these methods getNewId() & fetchIdsInReserve() thread safe ?

public final class IdManager {

    private static final int NO_OF_USERIDS_TO_KEEP_IN_RESERVE = 200;

    private static final AtomicInteger regstrdUserIdsCount_Cached = new AtomicInteger(100);
    private static int noOfUserIdsInReserveCurrently = 0;

    public static int getNewId(){                   
              if (noOfUserIdsInReserveCurrently <= 20)

          return regstrdUserIdsCount_Cached.incrementAndGet();

    private static synchronized void fetchIdsInReserve(){
        int reservedInDBTill = DBCountersReader.readCounterFromDB(....); // read column from DB 

        if (noOfUserIdsInReserveCurrently + regstrdUserIdsCount_Cached.get() != reservedInDBTill) throw new Exception("Unreserved ids alloted by app before reserving from DB");

        if (DBUpdater.incrementCounter(....)) //if write back to DB is successful
              noOfUserIdsInReserveCurrently += NO_OF_USERIDS_TO_KEEP_IN_RESERVE;


2 Answers 2


You may still have a logic problem. The first time getNewId() is called noOfUserIdsInReserveCurrently = 0 so fetchIdsInReserve() is called and noOfUserIdsInReserveCurrently = 199.

Now imagine the thread gets parked before returning.

Now imagine this happens to 179 more threads. So that we are now at noOfUserIdsInReserveCurrently = 20 and triggering fetchIdsInReserve() again. at this point there is no guarantee that return regstrdUserIdsCount_Cached.incrementAndGet() has ever been called so regstrdUserIdsCount_Cached.get() = 0.

Given this, does the check if (noOfUserIdsInReserveCurrently + regstrdUserIdsCount_Cached.get() != reservedInDBTill) fail?

This is not an issue if there are a fixed number of threads that can call this, like from a FixedThreadPool. But if it is unconstrained then there might be a failure mode.

  • \$\begingroup\$ Thankyou so much! To solve this issue, should I include regstrdUserIdsCount_Cached inside the synchronized block & make it as a non final int instead of atomicInteger ? \$\endgroup\$ Commented Apr 3, 2012 at 19:48
  • \$\begingroup\$ To be honest I'm not sure what you are trying to accomplish. But synchronizing the getNewId() method would make the whole system safe. \$\endgroup\$
    – Clint
    Commented Apr 4, 2012 at 3:36

Imagine this scenario: thread A arrives first and executes the entire synchronized(IdManager.class) block, but is interrupted before the row return regstrdUserIdsCount_Cached.incrementAndGet(); so that the counter is not incremented.

The arrives thread B. It enters the synchronized(IdManager.class) block and gets the same Id as thread A!!!

This could happen because you have put the regstrdUserIdsCount_Cached.incrementAndGet(); outside the syncronized block. If you put it inside, your code should be thread safe.


The scenario described above is not possibile, because incrementAndGet is an atomic operation. Indeed, the code is thread-safe. My apologies.

  • \$\begingroup\$ Wanted to add that you might as well synchronize the method. When you synchronize(IdManager.class), you prevent any other thread from accessing any other synchronized static methods for that class. \$\endgroup\$ Commented Apr 1, 2012 at 20:55
  • \$\begingroup\$ I disagree with your reasoning, @vitalik. The decision on what ID to return to a thread is based solely on the result of regstrdUserIdsCount_Cached.incrementAndGet() - this is not influenced by the reservation logic, so the code is in fact threadsafe. \$\endgroup\$
    – sgmorrison
    Commented Apr 1, 2012 at 21:02
  • \$\begingroup\$ I guess you are right @sgmorrison. I corrected my answer. \$\endgroup\$ Commented Apr 1, 2012 at 21:08
  • \$\begingroup\$ @ApprenticeQueue: In that case, since I am calling a synchronized method(fetchIdsInReserve()) from a synchronized block that could lead to a deadlock, right? \$\endgroup\$ Commented Apr 2, 2012 at 5:29
  • \$\begingroup\$ @Raj no deadlock because Java uses reentrant synchronization; if you have the lock already, you don't need to acquire the same lock again. \$\endgroup\$ Commented Apr 2, 2012 at 16:59

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.