workjava

Avoid synchronizing the class object returned by getClass()

I accidentally found some code these days and it occurs seems a bit suspicious.

Let’s take a look.

1
2
3
4
5
if ((UUID == null) || UUID.plus(1, Time.HOURS).isBefore(currentInstant)) {
    synchronized (getClass()) {
        UUID = currentInstant;
    }
}

The suspicious line is:

1
synchronized (getClass())

In Java, there’s a method called getClass() that tells you the type of an object. But using it to lock or synchronize the code might cause unexpected issues:

Some facts:

  • When you have a class and create a subclass, each class has its own “type object” that represents its type.

  • If you use getClass() in the subclass to lock, it locks on the subclass’s type object, not the parent class’s type object.

  • Synchronized code using getClass() may not work as you expect with subclasses.

  • The subclass will lock on its own type object, which may not be what you intended.

So, don’t use the type object from getClass() for synchronization.

See, more useful example (from here):

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
class Base {
  static DateFormat format =
      DateFormat.getDateInstance(DateFormat.MEDIUM);
 
  public Date parse(String str) throws ParseException {
    synchronized (getClass()) {
      return format.parse(str);
    }
  }
}
 
class Derived extends Base {
  public Date doSomethingAndParse(String str) throws ParseException {
    synchronized (Base.class) {
      // ...
      return format.parse(str);
    }
  }
}

If you want to synchronize on a class itself (using the class name), make sure that the lock object you use is not accessible to untrusted code. If your class is only used within a specific package, you can safely synchronize on its lock object.

Possible scenario

Imagine a hotel booking system that allows customers to search and book hotel rooms online. To ensure data consistency and prevent conflicts, the developer named John decides to synchronize the booking process using synchronized blocks.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
public class HotelBookingSystem {
    private Map<String, Room> rooms;

    public HotelBookingSystem() {
        rooms = new HashMap<>();
    }

    public synchronized void bookRoom(Room room, Customer customer) {
        synchronized (room.getClass()) {
            // some logic
            // ...
        }
    }
}

However, when implementing the synchronization, John mistakenly uses getClass() to lock the booking code. John believes that this will ensure exclusive access to the booking functionality based on the class type of the hotel room.

Now, let’s say that a few months later, the management decides to introduce a new type of room called “LuxuryRoom,” which extends the existing “Room” class. Bob, another developer on the team, is assigned the task of creating the subclass but is unaware of the synchronization issue present in the existing code.

Without being aware of the problem, Bob creates the LuxuryRoom subclass by extending the existing Room class. However, he forgets to override the synchronized booking methods inherited from the parent class.

In this situation, the problem arises. When multiple users concurrently try to book different types of rooms (e.g., both “Room” and “LuxuryRoom” objects), the synchronization block using getClass() will lock on the respective subclass’s type object instead of the parent class’s type object.

As a result, users trying to book different types of rooms may experience delays or conflicts. The synchronization block, locking based on the subclass’s type object, will prevent concurrent bookings across different room types, potentially leading to a degraded user experience and loss of business.

In summary, using getClass() for synchronization can cause unexpected problems, so be careful.

References:

Share