Base Mutex implementation on Semaphore and improve the latter #3016
Conversation
ndkoval
force-pushed
the
new-channels-mutex
branch
from
84709b8 to
882a827
Compare
|
Only those tests that require |
ndkoval
force-pushed
the
new-channels-mutex
branch
from
882a827 to
9658d3f
Compare
There was a problem hiding this comment.
By the way, you can reuse nom-intrusive implementation of onLock: 7755edb#diff-86e8b6879026f347540a4b86b2c594fe25128a0fb168cc0689ef48008c8b4d5fR99
|
@qwwdfsad I already have a proper implementation with new |
| affected._state.compareAndSet(this, update) | ||
| override fun resume(value: Unit, onCancellation: ((cause: Throwable) -> Unit)?) { | ||
| [email protected] = owner | ||
| cont.resume(value, onCancellation) |
There was a problem hiding this comment.
What will happen if this resume will have no effect, e.g. because the continuation was concurrently cancelled?
| @@ -52,24 +54,35 @@ public interface Mutex { | |||
| * Note that this function does not check for cancellation when it is not suspended. | |||
| * Use [yield] or [CoroutineScope.isActive] to periodically check for cancellation in tight loops if needed. | |||
| * | |||
| * Use [tryLock] to try acquiring a lock without waiting. | |||
| * This function can be used in [select] invocation with [onLock] clause. | |||
There was a problem hiding this comment.
This part is irrelevant as onLock is being deprecated for removal
| ) : CancellableContinuation<Unit> by cont { | ||
| override fun tryResume(value: Unit, idempotent: Any?, onCancellation: ((cause: Throwable) -> Unit)?): Any? { | ||
| val token = cont.tryResume(value, idempotent, onCancellation) | ||
| if (token !== null) [email protected] = owner |
There was a problem hiding this comment.
Please add a comment why it is safe to have a race here -- the continuation can be already executing and even invoking unlock while owner is not set
There was a problem hiding this comment.
It cannot be executing before completeResume(..)
| override fun toString(): String = "Empty[$locked]" | ||
| } | ||
| internal class MutexImpl(locked: Boolean) : SemaphoreImpl(1, if (locked) 1 else 0), Mutex { | ||
| private val owner = atomic<Any?>(if (locked) null else NO_OWNER) |
There was a problem hiding this comment.
Please document possible state transitions here and their purpose
|
Thanks for the review -- I have fixed and clarified everything in #3020 |
No description provided.