• View
• Edit
• History
• Print

Notes about notifyAll() in DEDirector

We started getting IllegalMonitorStateException when calling DEDirector.fireAt(). Below are details.

Replication from the command line

Messages like the following will appear:

Note that the problem does not occur when running make in ptolemy/actor/lib/jjs/modules/socket/test/, it only occurs when running in ptolemy/actor/lib/jjs/modules/socket/test/junit.

Replicating In Eclipse

Start a new Java Application configuration

• Man tab:
  • Project: ptII
  • Main class: org.junit.runner.JUnitCore
• Arguments tab:
  • Program Arguments: ptolemy.actor.lib.jjs.modules.socket.test.junit.JUnitTclTest
  • Working Directory. Check Other and fill in: ${workspace_loc:ptII}/ptolemy/actor/lib/jjs/modules/socket/test

Edit DEDirector and change:

and set a breakpoint at the throw ex; line

Below is the stack trace that I get:

FindBugs notifyAll() Warning

FindBugs does complain about notifyAll() at that location:

The call to notifyAll() appears in DEDirector.fireAt():

The call to _enqueueEvent(actor, result, index); also does:

So, fireAt() gets the lock, then calls _enqueueEvent() which also gets the lock.

Coverity Scan Warnings

Summary: Checking the Coverity Scan Warnings at http://scan.coverity.com shows two other warnings for DEDirector, but they are probably not relevant.

Details: There are two warnings

One warning is:

 CID 1250909 (#1 of 1): Unguarded write (GUARDED_BY_VIOLATION)63. missing_lock: Accessing _exceedStopTime without holding lock DEDirector._eventQueue. lsewhere, "ptolemy.domains.de.kernel.DEDirector._exceedStopTime" is accessed with DEDirector._eventQueue held 3 out of 4 times.

The offending line in DEDirector._getNextActorToFire() is marked below:

_getNextActorToFire() is called at the top of fire(), so this is probably not the issue.

Another Coverity Scan warning is:

  CID 1250851 (#1 of 1): Check of thread-shared field evades lock acquisition (LOCK_EVASION)9. thread1_overwrites_value_in_field: Thread1 sets _microstep to a new value. Now the two threads have an inconsistent view of _microstep and updates to fields correlated with _microstep may be lost.

The above also occurs in _getNextActorToFire(), so it is probably also not the issue here.

JUnit

Oddly, running make in ptolemy/actor/lib/jjs/modules/socket/test does not cause the problem,

but running make in in ptolemy/actor/lib/jjs/modules/socket/test/junit does.

Summary: The problem occurs with the second junit run of Message2.xml Message3.xml, Socket3.xml

Details below.

JUnit Failure #1

A typical failure is

JUnit Failure #2

JUnit Failure #3

JUnit Failure #4

JUnit Failure #5

Later, in the same run:

Possible Solution

Edward wrote:

This is a really weird one...

The only reason I can think of for the findBugs warning is that findBugs may believe that the _eventQueue field could change between when the synchronization lock is acquired and when notifyAll is called.

This would only occur if a second execution of the model actually overlaps in time with the first... I guess this is theoretically possible, and may in fact be happening in the test mechanism that is triggering the bug.

One possible fix might be in preinitialize. Instead of:

we could do:

Another alternative would be to build a mechanism for the second execution to wait until the first has completely wrapped up. That could be tricky, however, as there may be external threads attempting to queue events even after wrapup is called.

Possible Solution Continued

Unfortunately, that means that we are changing the value of the lock while holding the lock. FindBugs warns:

Even worse, PtidesDirector.preinitialize() looks like:

PtidesDirector should be using synchronized() in many places...

I see these options:

1. Use a different object to lock the _eventQueue
2. Make _eventQueue final. This fails because DEDirector.clone() and PtidesDirectory.preinitialize() set _eventQueue
3. In DEDirector.preinitialize(), add a method that would update the fields of _eventQueue without changing _eventQueue itself.

The problem with #3 is that _eventQueue() is declared as a DEEventQueue, but we instantiate a DECQEventQueue(). So we can't just create DECQEventQueue.initialize(int minBinCount, int binCountFactor, boolean isAdaptive) and invoke it in DEDirector.preinitialize() because _eventQueue is declared as a DEEventQueue().

In DEDirector.preinitialize(), we could do an instanceof check, but this seems wrong, especially because PtidesDirector.preinitialize() is setting _eventQueue to a new PtidesListEventQueue after the call to DEDirector.preinitialize()

So, I'm thinking that we need to add a lock, which I've done, which solves the FindBugs warnings and passes the test in ptolemy/actor/lib/jjs/modules/socket/test/junit

See Also

• Threading And Discrete Event Models - summary
• Vert.x - Info about building and debugging Vert.x
• WebSocket Deadlock - After the above fixes for notifyAll(), actor/lib/jjs/modules/webSocket/test/auto/FullDuplex.xml and FullDuplex2 are hanging