Reentrantlock works fine in Java but causes IllegalMonitorException in Scala

Question

I would like to migrate a Java function

protected static final Lock LOCK = new ReentrantLock();
public double calculate(...){
    try {
        LOCK.tryLock(20, TimeUnit.SECONDS);
        ...
    }finally{
        LOCK.unlock()
    }
}

The same function in Scala:

protected final def LOCK = new ReentrantLock
def calculate(...): double = {
    try{
        LOCK.tryLock(20, TimeUnit.Seconds)
        ...
    }finally{
        LOCK.unlock()
    }
}

The LOCK.unlock() is always causing a IllegalMonitorStateException. I dont see any reason why this is happpening.

Can someone tell me where the problem is?

Is your `calculate` method in a class/trait by any chance? Because in the java version `Lock` is static, so translating that in scala would mean putting it in an object, **not** in the class/trait where `calculate` is defined. — Régis Jean-Gilles, Apr 08 '16 at 08:52
Plus, and this is probably the reason of the error, you are defining `LOCK ` as a `def` but it should really be a `val`. Otherwise, you are recreating a new lock each time you reference `LOCK` effectively rendering the lock useless. — Régis Jean-Gilles, Apr 08 '16 at 08:55
Common practice is to acquire a lock before try, not inside. — Alex Salauyou, Apr 08 '16 at 08:56
the calculate method is in a abstract class. Is is maybe because an other thread is trying to unlock this LOCK ? — wurmi, Apr 08 '16 at 09:01

Régis Jean-Gilles · Accepted Answer · 2016-04-08T09:39:45.243

You should definitely make LOCK a val instead of a def. As it stands, you are recreating a new instance of ReetrantLock every time you. Effectively what you are doing is this:

try {
    // Useless as we are creating a new lock
    (new ReentrantLock).tryLock(20, TimeUnit.SECONDS).tryLock(20, TimeUnit.SECONDS).tryLock(20, TimeUnit.SECONDS);
    ...
}finally{
    // Useless too, and will actually throw because we unlock a fresh (thus unlocked) lock
    (new ReentrantLock).unlock()
}

It is obviously bound to fail.

You should do something like:

object MyClass {
  private val LOCK = new ReentrantLock
}
class MyClass {
  def calculate(...): double = {
      try{
          LOCK.tryLock(20, TimeUnit.Seconds)
          ...
      }finally{
          LOCK.unlock()
      }
  }
}

Which is the direct translation to scala of your original java code.

Finally, in his (now deleted) answer Jon Skeet rightly suggests:

You should only unlock the lock if you managed to acquire it - and the conventional pattern is to put the lock/tryLock call before the try. (It doesn't matter with tryLock(), but it does matter for lock(), so we might as well be consistent.)

Which gives:

object MyClass {
  private val LOCK = new ReentrantLock
}
class MyClass {
  def calculate(...): double = {
      val gotLock = LOCK.tryLock(20, TimeUnit.Seconds)
      try {
          ...
      } finally {
        if (gotLock) {
          LOCK.unlock()
        }
      }
  }
}

LOCK cannot be resolved in 'MyClass'. Seems that I am not missing something — wurmi, Apr 08 '16 at 09:14

Reentrantlock works fine in Java but causes IllegalMonitorException in Scala

1 Answers1