Fix context lock removal

[hubertp]

Pull Request Description

Removal of context lock assumed that one still holds a lock on it. This is no longer the case when using a withContextLock block that correctly manages the resource. This change fixes the IllegalMonitorStateException.
Closes #10254.

Checklist

Please ensure that the following checklist has been satisfied before submitting the PR:

• The documentation has been updated, if necessary.
• Screenshots/screencasts have been attached, if there are any visual changes. For interactive or animated visual changes, a screencast is preferred.
• All code follows the
  Scala,
  Java,
  TypeScript,
  and
  Rust
  style guides. In case you are using a language not listed above, follow the Rust style guide.
• Unit tests have been written where possible.

[JaroslavTulach]

The context lock is automatically created by withContextLock - why it is not cleaned at the end of the withContextLock method? That's the intended behavior of withXyz methods - we should delete the removeContextLock method.

[JaroslavTulach]

I don't understand the desired behavior. Why do we have removeContextLock at all? Because of this line? Then just remove it!

[hubertp]

Yes. I suppose it was added to avoid creating new instances of ReentrantLock. Once the lock is created it is re-used. It saves roughly 10ms per command (as that particular lock is required quite often).

[JaroslavTulach]

Everywhere, where ctx.locking.withContextLock( is used, there is an instance of RuntimeContext. To

avoid creating new instances of ReentrantLock

the RuntimeContext shall obtain some object instance via ctx.locking.createContextLock and then use it via ctx.locking.withContextLock( until it is needed. At the end it should close it (removeContextLock).

[JaroslavTulach]

Possible API could look like:

diff --git engine/runtime-instrument-common/src/main/java/org/enso/interpreter/instrument/execution/Locking.java engine/runtime-instrument-common/src/main/java/org/enso/interpreter/instrument/execution/Locking.java
index fc6e3f6389..b223ce34fb 100644
--- engine/runtime-instrument-common/src/main/java/org/enso/interpreter/instrument/execution/Locking.java
+++ engine/runtime-instrument-common/src/main/java/org/enso/interpreter/instrument/execution/Locking.java
@@ -37,21 +37,25 @@ public interface Locking {
   <T> T withPendingEditsLock(Class<?> where, Callable<T> callable);
 
   /**
-   * Executes `callable` while holding a context lock
+   * Allocates new handle for use in {@link #withContextLock} associated with given context ID.
    *
-   * @param contextId context id for which the lock is being requested
-   * @param where the class requesting the lock
-   * @param callable code to be executed while holding the lock
-   * @return the result of calling `callable` or null, if no result is expected
+   * @param contextId context ID
+   * @return an object to pass to {@link #withContextLock} and to {@link AutoCloseable#close()} when
+   *     no longer needed
    */
-  <T> T withContextLock(UUID contextId, Class<?> where, Callable<T> callable);
+  AutoCloseable createContextLock(UUID contextId);
 
   /**
-   * Removes a context lock.
+   * Executes `callable` while holding a context lock
    *
-   * @param contextId a context to remove
+   * @param contextLockHandle handle created by {@link #createContextLock(UUID)}
+   * @param where the class requesting the lock
+   * @param callable code to be executed while holding the lock
+   * @return the result of calling `callable` or null, if no result is expected
+   * @throws ClassCastException if {@code contextLockHandle} isn't coming from {@link
+   *     #createContextLock(UUID)}
    */
-  void removeContextLock(UUID contextId);
+  <T> T withContextLock(AutoCloseable contextLockHandle, Class<?> where, Callable<T> callable);
 
   /**
    * Executes `callable` while holding a file lock

More advanced tricks with generics (to hide ClassCastException from the docs) are possible, but probably unnecessary.

[JaroslavTulach]

Hopefully the AutoCloseable solution will be easy to implement.

[hubertp]

Tried to address your comment @JaroslavTulach, didn't really see the need for AutoCloseable. If still not good, please feel free to send a patch to this PR

[JaroslavTulach]

If still not good, please feel free to send a patch to this PR

More encapsulation in c8b5111. Feel free to go with or without c8b5111 change.

[JaroslavTulach]

Unblocking.