Expiration: Fix race with GC when handling already disposed Disposable

On a rare occasion an instance of DisposableExpiration created for an
already disposed Disposable could have been left non-expired. This used
to happen when the GC had already collected a weakly-referenced child
disposable by the time we disposed the instance of
eakReferenceDisposableWrapper referencing the child.

This also fixes
NonBlockingReadActionTest.testSyncExecutionThrowsPCEWhenExpired failing
randomly.

GitOrigin-RevId: 28dfeef0e7d1ef501db0554a5d8069f82603211d
This commit is contained in:
Eldar Abusalimov
2019-11-07 02:40:58 +00:00
committed by intellij-monorepo-bot
parent 2ad1a76771
commit ef177e7a04
2 changed files with 10 additions and 1 deletions
@@ -5,6 +5,7 @@ import com.intellij.openapi.Disposable
import com.intellij.openapi.WeakReferenceDisposableWrapper
import com.intellij.openapi.util.Disposer
import com.intellij.util.IncorrectOperationException
import com.intellij.util.ObjectUtils
import kotlinx.coroutines.*
/**
@@ -146,6 +147,7 @@ internal fun Disposable.cancelJobOnDisposal(job: Job,
if (!tryRegisterDisposable(this, childRef)) {
Disposer.dispose(childRef) // runs disposableBlock()
ObjectUtils.reachabilityFence(child)
return AutoCloseable { }
}
else {
@@ -12,7 +12,7 @@ import kotlinx.coroutines.Job
class ExpirationTest : LightPlatformTestCase() {
private fun createTestDisposable(): Disposable {
return Disposable { }.also { Disposer.register(testRootDisposable, it) }
return Disposer.newDisposable(name).also { Disposer.register(testRootDisposable, it) }
}
private fun createCompositeExpiration(vararg expiration: Expiration): Expiration {
@@ -20,6 +20,13 @@ class ExpirationTest : LightPlatformTestCase() {
return Expiration.composeExpiration(expiration.toList())!!
}
fun `test isExpired for already disposed Disposable`() {
val disposable = createTestDisposable()
Disposer.dispose(disposable)
val expiration = DisposableExpiration(disposable)
assertTrue("Expiration.isExpired must be true", expiration.isExpired)
}
fun `test isExpired for JobExpiration`() {
val job = Job()
val expiration = JobExpiration(job)