Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reduce reachable references of disposed invokeOnTimeout handle #3353

Merged
merged 2 commits into from Jun 30, 2022
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
22 changes: 19 additions & 3 deletions kotlinx-coroutines-core/native/src/MultithreadedDispatchers.kt
Expand Up @@ -35,9 +35,25 @@ internal class WorkerDispatcher(name: String) : CloseableCoroutineDispatcher(),
}

override fun invokeOnTimeout(timeMillis: Long, block: Runnable, context: CoroutineContext): DisposableHandle {
// No API to cancel on timeout
worker.executeAfter(timeMillis.toMicrosSafe()) { block.run() }
return NonDisposableHandle
// Workers don't have an API to cancel sent "executeAfter" block, but we are trying
// to control the damage and reduce reachable objects by nulling out `block`
// that may retain a lot of references, and leaving only an empty shell after a timely disposal
// This is a class and not an object with `block` in a closure because that would defeat the purpose.
class DisposableBlock(block: Runnable) : DisposableHandle, Function0<Unit> {
qwwdfsad marked this conversation as resolved.
Show resolved Hide resolved
private val disposableHolder = AtomicReference<Runnable?>(block)

override fun invoke() {
disposableHolder.value?.run()
qwwdfsad marked this conversation as resolved.
Show resolved Hide resolved
}

override fun dispose() {
disposableHolder.value = null
}
}

val disposableBlock = DisposableBlock(block)
worker.executeAfter(timeMillis.toMicrosSafe(), disposableBlock)
return disposableBlock
Comment on lines +54 to +56
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
val disposableBlock = DisposableBlock(block)
worker.executeAfter(timeMillis.toMicrosSafe(), disposableBlock)
return disposableBlock
DisposableBlock(block).also { worker.executeAfter(timeMillis.toMicrosSafe(), it) }

The variable name does not seem to be informative.

}

override fun close() {
Expand Down