JDK-6560119 : incorrect synchonization in com/sun/jmx/remote/internal/ArrayNotificationBuffer.java
  • Type: Bug
  • Component: core-svc
  • Sub-Component: javax.management
  • Affected Version: 6u1
  • Priority: P4
  • Status: Resolved
  • Resolution: Fixed
  • OS: generic
  • CPU: generic
  • Submitted: 2007-05-21
  • Updated: 2010-07-29
  • Resolved: 2007-06-10
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 6 JDK 7
6u4 b01Fixed 7Fixed
Description
In the Java6 JDK, file:   src/share/classes/com/sun/jmx/remote/internal/ArrayNotificationBuffer.java

void removeSharer(xxx) {
  sync(this) {
    empty = ...; // set empty=true
  }              // UNLOCK 'this' - 1
  if( empty )    // test empty - 2
    dispose();   // call dispose
}

public void dispose() {
  synchronized(this) {  // RELOCK 'this' - 3
    ...
  }
}

So basically the order of events is:
- 1 : UNLOCK 'this'
- 2 : test empty
- 3 : RELOCK 'this'

If another thread invalidates the 'empty' flag while at step 2 ('this' is unlocked), e.g. by calling addSharer on the same ArrayNotificationBuffer then you end up calling 'dispose()' on a non-empty sharer.  Our stress-mode -Xcomp JCK test runs trigger this crash fairly routinely.
Comments
EVALUATION I have reworked and simplified the synchronization logic in ArrayNotificationBuffer and added comments to document more clearly how it works. This problem should no longer exist (subject to confirmation by the previously-failing JCK tests).
25-05-2007
EVALUATION The logic is indeed incorrect. The reason the call to dispose() is outside the synchronized block is that we do not want to hold any locks while we are removing the listeners that the buffer has on user MBeans. Removing those listeners involves calling user code so it would be risky to hold any locks.
22-05-2007