Synchronizing On The Instance Itself

ID

java.no_synchronize_on_this

Severity

high

Remediation Complexity

trivial

Remediation Risk

low

Remediation Effort

low

Resource

Reliability

Language

Java

Tags

concurrency, reliability

Description

Reports a synchronized block that locks on the enclosing instance — written either as synchronized (this), or as synchronized (Outer.this) from an inner class.

The intrinsic monitor of an object is part of its public surface. Any code holding a reference to the instance can take the same lock, so the class cannot reason about who contends for its own critical sections.

Rationale

class TicketCounter {
  private int issued;

  void issueTicket() {
    synchronized (this) {          // the monitor is reachable from outside
      issued++;
    }
  }
}

Every caller that can see a TicketCounter can also write synchronized (counter) { … }. From there:

  • unrelated code can hold the lock for as long as it likes and stall issueTicket indefinitely;

  • it can take the lock and then call back into the counter, turning a bounded critical section into one that depends on foreign code;

  • it can take this lock and another one in the opposite order and deadlock the pair.

None of that is visible from inside the class, which is what makes the failure hard to diagnose: it shows up as an intermittent hang with no obvious owner.

The rule deliberately leaves four shapes alone:

  • synchronized methods. The implicit lock is the same object, but the declaration is such a widespread idiom that reporting it would drown the finding that matters.

  • synchronized (SomeType.class) and synchronized (getClass()), where the shared object is the class rather than the instance.

  • Locks on fields, parameters and locals, which fail for different reasons and are covered by their own rules.

  • synchronized (this) { … wait(); … } and the notify/notifyAll counterpart. Object.wait and Object.notify may only be called by the thread that owns that object’s monitor, so locking on anything else there throws IllegalMonitorStateException. The exemption needs the wait or notify to be made on the locked object itself: synchronized (this) { synchronized (lock) { lock.notifyAll(); } } notifies a different object, so the lock on this is still reported.

Remediation

Lock on a private final field the class owns and uses for nothing else. The set of code that can hold it is then exactly the code in the file:

class TicketCounter {
  private final Object gate = new Object();
  private int issued;

  void issueTicket() {
    synchronized (gate) {
      issued++;
    }
  }
}

java.util.concurrent.locks.ReentrantLock is an equally good choice when the critical section needs timed or interruptible acquisition.

References

© 2026 Xygeni. All rights reserved