From 368fd7328b863ac3d6096234565964f89e98a191 Mon Sep 17 00:00:00 2001 From: Jaroslav Tulach Date: Tue, 10 Oct 2023 07:26:50 +0200 Subject: [PATCH 1/2] Define and enforce locking order during runtime --- .../execution/ReentrantLocking.scala | 103 ++++++++++++++++-- 1 file changed, 96 insertions(+), 7 deletions(-) diff --git a/engine/runtime-instrument-common/src/main/scala/org/enso/interpreter/instrument/execution/ReentrantLocking.scala b/engine/runtime-instrument-common/src/main/scala/org/enso/interpreter/instrument/execution/ReentrantLocking.scala index 7b7cb450df0a..8cb4b0b5c4a5 100644 --- a/engine/runtime-instrument-common/src/main/scala/org/enso/interpreter/instrument/execution/ReentrantLocking.scala +++ b/engine/runtime-instrument-common/src/main/scala/org/enso/interpreter/instrument/execution/ReentrantLocking.scala @@ -12,19 +12,25 @@ import java.util.logging.Level */ class ReentrantLocking(logger: TruffleLogger) extends Locking { - private val compilationLock = new ReentrantReadWriteLock(true) - + /** Lowest level lock obtainable at any time. */ private val pendingEditsLock = new ReentrantLock() - private val contextMapLock = new ReentrantLock() + /** Obtain anytime, except when holding pendingsEditLock. Guarded by fileMapLock. */ + private var fileLocks = Map.empty[File, ReentrantLock] + /** Lower than contextLocks. Higher than anything else. */ + private val compilationLock = new ReentrantReadWriteLock(true) + + /** The highest lock. Always obtain first. Guarded by contextMapLock. */ private var contextLocks = Map.empty[UUID, ReentrantLock] - private val fileMapLock = new ReentrantLock() + /** Guards contextLocks */ + private val contextMapLock = new ReentrantLock() - private var fileLocks = Map.empty[File, ReentrantLock] + /** Guards fileLocks */ + private val fileMapLock = new ReentrantLock() - protected def getContextLock(contextId: UUID): Lock = { + private def getContextLock(contextId: UUID): Lock = { contextMapLock.lock() try { if (contextLocks.contains(contextId)) { @@ -49,7 +55,7 @@ class ReentrantLocking(logger: TruffleLogger) extends Locking { } } - protected def getFileLock(file: File): Lock = { + private def getFileLock(file: File): Lock = { fileMapLock.lock() try { if (fileLocks.contains(file)) { @@ -76,6 +82,16 @@ class ReentrantLocking(logger: TruffleLogger) extends Locking { /** @inheritdoc */ override def acquireWriteCompilationLock(): Long = { + assertNotLocked( + compilationLock, + true, + "Cannot upgrade compilation read lock to write lock" + ) + assertNotLocked( + pendingEditsLock, + s"Cannot acquire compilation write lock when having pending edits lock" + ) + assertNoFileLock("Cannot acquire write compilation lock") logLockAcquisition(compilationLock.writeLock(), "write compilation") } @@ -85,6 +101,16 @@ class ReentrantLocking(logger: TruffleLogger) extends Locking { /** @inheritdoc */ override def acquireReadCompilationLock(): Long = { + // CloseFileCmd does: + // ctx.locking.acquireReadCompilationLock() + // ctx.locking.acquireFileLock(request.path) + assertNoFileLock("Cannot acquire read compilation lock") + // CloseFileCmd also adds: + // ctx.locking.acquirePendingEditsLock() + assertNotLocked( + pendingEditsLock, + s"Cannot acquire compilation read lock when having pending edits lock" + ) logLockAcquisition(compilationLock.readLock(), "read compilation") } @@ -103,6 +129,21 @@ class ReentrantLocking(logger: TruffleLogger) extends Locking { /** @inheritdoc */ override def acquireContextLock(contextId: UUID): Long = { + assertNotLocked( + compilationLock, + true, + s"Cannot acquire context ${contextId} lock when having compilation read lock" + ) + assertNotLocked( + compilationLock, + false, + s"Cannot acquire context ${contextId} lock when having compilation write lock" + ) + assertNoFileLock(s"Cannot acquire context ${contextId}") + assertNotLocked( + pendingEditsLock, + s"Cannot acquire context ${contextId} lock when having pending edits lock" + ) logLockAcquisition(getContextLock(contextId), s"$contextId context") } @@ -112,6 +153,12 @@ class ReentrantLocking(logger: TruffleLogger) extends Locking { /** @inheritdoc */ override def acquireFileLock(file: File): Long = { + // cannot have pendings lock as of EnsureCompiledJob.applyEdits + assertNotLocked( + pendingEditsLock, + s"Cannot acquire file ${file} lock when having pending edits lock" + ) + assertNoContextLock(s"Cannot acquire file ${file}") logLockAcquisition(getFileLock(file), "file") } @@ -126,4 +173,46 @@ class ReentrantLocking(logger: TruffleLogger) extends Locking { now2 } + private def assertNotLocked( + lock: ReentrantReadWriteLock, + read: Boolean, + msg: String + ) = { + val locked = + if (read) lock.getReadHoldCount() > 0 + else lock.isWriteLockedByCurrentThread() + if (locked) { + throw new IllegalStateException(msg) + } + } + private def assertNotLocked(lock: ReentrantLock, msg: String) = { + if (lock.isHeldByCurrentThread()) { + throw new IllegalStateException(msg) + } + } + + private def assertNoFileLock(msg: String) = { + fileMapLock.lock() + try { + for (ctx <- fileLocks) { + assertNotLocked(ctx._2, msg + s" when having file ${ctx._1} lock") + } + } finally { + fileMapLock.unlock() + } + } + + private def assertNoContextLock(msg: String) = { + contextMapLock.lock() + try { + for (ctx <- contextLocks) { + assertNotLocked( + ctx._2, + msg + s" lock when having context ${ctx._1} lock" + ) + } + } finally { + contextMapLock.unlock() + } + } } From e16b1e3337218c20ee2e2868078d90cf77d4272e Mon Sep 17 00:00:00 2001 From: Jaroslav Tulach Date: Tue, 10 Oct 2023 07:27:14 +0200 Subject: [PATCH 2/2] Recover when a logger isn't available --- .../command/SynchronousCommand.scala | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/engine/runtime-instrument-common/src/main/scala/org/enso/interpreter/instrument/command/SynchronousCommand.scala b/engine/runtime-instrument-common/src/main/scala/org/enso/interpreter/instrument/command/SynchronousCommand.scala index b6919b808909..a94f577cacba 100644 --- a/engine/runtime-instrument-common/src/main/scala/org/enso/interpreter/instrument/command/SynchronousCommand.scala +++ b/engine/runtime-instrument-common/src/main/scala/org/enso/interpreter/instrument/command/SynchronousCommand.scala @@ -24,13 +24,19 @@ abstract class SynchronousCommand(maybeRequestId: Option[RequestId]) } catch { case _: InterruptedException | _: ThreadInterruptedException => Interrupted - case ex: Throwable => - logger.log( - Level.SEVERE, - s"An error occurred during execution of $this command", - ex - ) + case ex: Throwable => { + val msg = s"An error occurred during execution of $this command" + try { + logger.log(Level.SEVERE, msg, ex) + } catch { + case ise: IllegalStateException => + // Thread using TruffleLogger has to have a current context or the TruffleLogger has to be bound to an engine + ex.printStackTrace() + ise.initCause(ex) + throw ise + } Done + } } finally { logger.log(Level.FINE, s"Command $this finished.") }