What can cause IllegalMonitorStateException from inside a synchronized block?

Question

We hit an extremely surprising exception today. Inside of a synchronized block, we call wait() and it throws IllegalMonitorStateException. What can cause this?

This is happening in well-tested open source code: http://svn.apache.org/viewvc/river/jtsk/trunk/src/com/sun/jini/jeri/internal/mux/Mux.java?view=markup#l222

We eliminated the obvious causes:

• are we synchronized on the right variable? Yes, it's muxLock
• is it a mutable variable? No, muxLock is final
• are we using any weird "-XX:" JVM flags that might affect monitor behavior? No, but we are launching the JVM embedded inside a C++ app via JNI.
• is this a strange JVM? No, it's Sun's 1.6.0_25 win/x64 JRE
• is this a known JVM bug? Can't find anything relevant at http://bugs.sun.com/bugdatabase

So, I'm trying to think of more far-fetched explanations.

• could an uncaught out-of-memory error cause the monitor state to be screwed up? We're looking at this, but we're seeing no evidence of memory errors yet.

UPDATE: (based on comment)

I've also verified from the stacktrace and breakpoint that the thread is indeed inside the synchronized block when the exception is thrown. It's not the case that some other unrelated code is emitting the exception (unless something is REALLY confusing Eclipse!)

Answer 1

The only suspicious thing I see that you are passing a reference to 'this' to some other object in your constructor. Is it possible (in fact, not unlikely) that, through weird re-ordering of things, if some other thread gets that reference to 'this' and calls the method that uses the muxlock, things can go extremely wrong.

The Java Language Specification is pretty specific about this:

An object is considered to be completely initialized when its constructor finishes. A thread that can only see a reference to an object after that object has been completely initialized is guaranteed to see the correctly initialized values for that object's final fields.

In other words, if another thread gets hold of the 'this' reference before the constructor is finished, the final field 'muxlock' might not be correctly initialized yet. In general, publishing a reference to 'this' before the constructor has finished can be pretty dangerous, especially in threaded situations.

Some potentially useful discussion about such things: http://madpropellerhead.com/random/20100328-java-final-fields-are-not-as-final-as-you-may-think

For some older, but still useful general discussion of why publishing 'this' in a constructor is a very bad idea in general, see for instance: http://www.ibm.com/developerworks/java/library/j-jtp0618/index.html

Answer 2

http://svn.apache.org/viewvc/river/jtsk/trunk/src/com/sun/jini/jeri/internal/mux/Mux.java?r1=1069292&r2=1135026&diff_format=h

here i can see that timeout was added lately

make sure that startTimeout is > than 0 otherwise you will wait(0) or wait(-n) this probably cause IllegalMonitorStateException

EDIT: Ok above is a disaster But lets try this :

we are in Mux constructor : http://svn.apache.org/viewvc/river/jtsk/trunk/src/com/sun/jini/jeri/internal/mux/Mux.java?view=markup

line 176 we create SocketChannelConnectionIO andd pass this after that we break and and different thread takes over .

in constructor of SocketChannelConnectionIO defined here : http://svn.apache.org/viewvc/river/jtsk/trunk/src/com/sun/jini/jeri/internal/mux/SocketChannelConnectionIO.java?view=markup line 112 we register to channel with the new handler().

handler recieaves something on chanel and function let say function handleReadReady is executed we synchronize on muxLock .

now we are still in constructor so object in final is still mutable !!! let assume it changes , now we have something waiting on different muxLock

One in a million scenario

EDIT

http://svn.apache.org/viewvc/river/jtsk/trunk/src/com/sun/jini/jeri/internal/mux/Mux.java?revision=1135026&view=co

Mux(SocketChannel channel,
    int role, int initialInboundRation, int maxFragmentSize)
    throws IOException
    {
    this.role = role;
    if ((initialInboundRation & ~0x00FFFF00) != 0) {
        throw new IllegalArgumentException(
        "illegal initial inbound ration: " +
        toHexString(initialInboundRation));
    }
    this.initialInboundRation = initialInboundRation;
    this.maxFragmentSize = maxFragmentSize;

    //LINE BELOW IS CAUSING PROBLEM it passes this to SocketChannelConnectionIO
    this.connectionIO = new SocketChannelConnectionIO(this, channel);

    //Lets assume it stops here we are still in constructor
    //and we are not in synchronized block

    directBuffersUseful = true;
    }

now in constructor of SocketChannelConnectionIO http://svn.apache.org/viewvc/river/jtsk/trunk/src/com/sun/jini/jeri/internal/mux/SocketChannelConnectionIO.java?revision=1069292&view=co

SocketChannelConnectionIO(Mux mux, SocketChannel channel)
    throws IOException
{
    super(mux);
    channel.configureBlocking(false);
    this.channel = channel;
    //Line below we are registering to the channel with mux that is still mutable
    //this is the line that actually is causing the problem move that to 
    // start() and it should work 
    key = selectionManager.register(channel, new Handler());
}

move this code to start() should work key = selectionManager.register(channel, new Handler()); (i am assuming start is executet when we want to start prosessing)

/**
 * Starts processing connection data.
 */
void start() throws IOException {
    key = selectionManager.register(channel, new Handler());
    key.renewInterestMask(SelectionKey.OP_READ);
}

But it would be much better not to create SocketChannelConnectionIO in the constructor of mux but maybe somewhere after that the same for second constructor creating StreamConnectionIO with this

Answer 3

The answer is in my opinion that its either a bug, or someone changed the object behind the reference despite its being final. If you can reproduce it, I recommend to set a read/write breakpoint on muxlock field to see if it is touched or not. You could check the identityhashcode of the muxlock in the first line of the synchronized block, and before waits and notifies with appropiate log entries or breakpoints. With reflection you can change final references. Quote from http://download.oracle.com/javase/6/docs/api/java/lang/reflect/Field.html:

"If the underlying field is final, the method throws an IllegalAccessException unless setAccessible(true) has succeeded for this field and this field is non-static. Setting a final field in this way is meaningful only during deserialization or reconstruction of instances of classes with blank final fields, before they are made available for access by other parts of a program. Use in any other context may have unpredictable effects, including cases in which other parts of a program continue to use the original value of this field."

Maybe its a bug in eclispe, and during debugging it somehow changes the field. Is it reproducable outside eclispe as well? Put a printstractrace in catch and see what happens.

Answer 4

Member variables are not as final as one would hope to. You should put the synchronized object into a final local variable, first. This does not explain why the member variable is altered, but if it fixes the problem you at least know that the member variable is really modified.