Skip to content

Commit 7b867fe

Browse files
ndkovalqwwdfsad
andauthored
Fix Mutex.tryLock() non-linearizability (#3781)
The scenario in question: - tryLock(owner), but the mutex was locked with the same owner - tryAcquire() fails - another thread releases the mutex - holdsLock(owner) fails, as the mutex is unlocked - another thread acquires the mutex with owner - isLocked returns true, and tryLock(owner) returns false. However, tryLock(o) should throw an exception. Fixes #3745 Signed-off-by: Nikita Koval <[email protected]> Co-authored-by: Vsevolod Tolstopyatov <[email protected]>
1 parent 0288b72 commit 7b867fe

File tree

1 file changed

+22
-12
lines changed
  • kotlinx-coroutines-core/common/src/sync

1 file changed

+22
-12
lines changed

kotlinx-coroutines-core/common/src/sync/Mutex.kt

Lines changed: 22 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -148,15 +148,22 @@ internal open class MutexImpl(locked: Boolean) : SemaphoreImpl(1, if (locked) 1
148148
override val isLocked: Boolean get() =
149149
availablePermits == 0
150150

151-
override fun holdsLock(owner: Any): Boolean {
151+
override fun holdsLock(owner: Any): Boolean = holdsLockImpl(owner) == HOLDS_LOCK_YES
152+
153+
/**
154+
* [HOLDS_LOCK_UNLOCKED] if the mutex is unlocked
155+
* [HOLDS_LOCK_YES] if the mutex is held with the specified [owner]
156+
* [HOLDS_LOCK_ANOTHER_OWNER] if the mutex is held with a different owner
157+
*/
158+
private fun holdsLockImpl(owner: Any?): Int {
152159
while (true) {
153160
// Is this mutex locked?
154-
if (!isLocked) return false
161+
if (!isLocked) return HOLDS_LOCK_UNLOCKED
155162
val curOwner = this.owner.value
156163
// Wait in a spin-loop until the owner is set
157164
if (curOwner === NO_OWNER) continue // <-- ATTENTION, BLOCKING PART HERE
158165
// Check the owner
159-
return curOwner === owner
166+
return if (curOwner === owner) HOLDS_LOCK_YES else HOLDS_LOCK_ANOTHER_OWNER
160167
}
161168
}
162169

@@ -187,16 +194,15 @@ internal open class MutexImpl(locked: Boolean) : SemaphoreImpl(1, if (locked) 1
187194
// The semaphore permit acquisition has failed.
188195
// However, we need to check that this mutex is not
189196
// locked by our owner.
190-
if (owner != null) {
191-
// Is this mutex locked by our owner?
192-
if (holdsLock(owner)) return TRY_LOCK_ALREADY_LOCKED_BY_OWNER
193-
// This mutex is either locked by another owner or unlocked.
194-
// In the latter case, it is possible that it WAS locked by
195-
// our owner when the semaphore permit acquisition has failed.
196-
// To preserve linearizability, the operation restarts in this case.
197-
if (!isLocked) continue
197+
if (owner == null) return TRY_LOCK_FAILED
198+
when (holdsLockImpl(owner)) {
199+
// This mutex is already locked by our owner.
200+
HOLDS_LOCK_YES -> return TRY_LOCK_ALREADY_LOCKED_BY_OWNER
201+
// This mutex is locked by another owner, `trylock(..)` must return `false`.
202+
HOLDS_LOCK_ANOTHER_OWNER -> return TRY_LOCK_FAILED
203+
// This mutex is no longer locked, restart the operation.
204+
HOLDS_LOCK_UNLOCKED -> continue
198205
}
199-
return TRY_LOCK_FAILED
200206
}
201207
}
202208
}
@@ -297,3 +303,7 @@ private val ON_LOCK_ALREADY_LOCKED_BY_OWNER = Symbol("ALREADY_LOCKED_BY_OWNER")
297303
private const val TRY_LOCK_SUCCESS = 0
298304
private const val TRY_LOCK_FAILED = 1
299305
private const val TRY_LOCK_ALREADY_LOCKED_BY_OWNER = 2
306+
307+
private const val HOLDS_LOCK_UNLOCKED = 0
308+
private const val HOLDS_LOCK_YES = 1
309+
private const val HOLDS_LOCK_ANOTHER_OWNER = 2

0 commit comments

Comments
 (0)