Move to centralised loom safe locks #8371
Conversation
…8290)" (square#8367) This reverts commit 689d388.
| internal object Locks { | ||
| inline fun <T> Dispatcher.withLock(action: () -> T): T { | ||
| contract { callsInPlace(action, InvocationKind.EXACTLY_ONCE) } | ||
| // TODO remove |
There was a problem hiding this comment.
Possibly temporary checks, unless we think these are cheap based on assertion enablement?
There was a problem hiding this comment.
Why do all the work to have a ReentrantLock and then forbid its use?
These checks are not fast enough to include.
https://publicobject.com/2019/11/18/kotlins-assert-is-not-like-javas-assert/
There was a problem hiding this comment.
It's the opposite, this was a quick check to fail if we also had synchronization on the owner, not a concurrent Lock.
|
|
||
| // TODO remove | ||
| assertThreadDoesntHoldLock() | ||
| // TODO can we assert we don't have the connection lock? |
There was a problem hiding this comment.
What's a good structure to handle this sort of lock hierarchy?
yschimke
added
android
|
Catching the ones I missed |
| var idleCallback: Runnable? = null | ||
| get() = this.withLock { field } |
There was a problem hiding this comment.
Maybe for follow up: should we do a property delegate so we can have syntax like this?
var idleCallback: Runnable? by lock.property(null)
(or similar)
| internal object Locks { | ||
| inline fun <T> Dispatcher.withLock(action: () -> T): T { | ||
| contract { callsInPlace(action, InvocationKind.EXACTLY_ONCE) } | ||
| // TODO remove |
There was a problem hiding this comment.
Why do all the work to have a ReentrantLock and then forbid its use?
These checks are not fast enough to include.
https://publicobject.com/2019/11/18/kotlins-assert-is-not-like-javas-assert/
Mostly #8290
But with a centralised lock definition. Draft for discussion, but possibly we could encode the rules from docs/contribute/concurrency.md, so we fail fast if we ever break them.