From fdb16c9f0e321b4fbd0a99aef877224270dec144 Mon Sep 17 00:00:00 2001 From: Jeff Gulbronson Date: Fri, 15 Mar 2024 08:21:10 -0400 Subject: [PATCH] Remove ActionScope.threadLocalUUID --- .../src/main/kotlin/misk/scope/ActionScope.kt | 35 +++++-------------- .../misk/scope/ActionScopePropagationTest.kt | 22 ++++++++---- 2 files changed, 24 insertions(+), 33 deletions(-) diff --git a/misk-action-scopes/src/main/kotlin/misk/scope/ActionScope.kt b/misk-action-scopes/src/main/kotlin/misk/scope/ActionScope.kt index 7452408b882..54cd72a7f9a 100644 --- a/misk-action-scopes/src/main/kotlin/misk/scope/ActionScope.kt +++ b/misk-action-scopes/src/main/kotlin/misk/scope/ActionScope.kt @@ -6,7 +6,6 @@ import jakarta.inject.Inject import jakarta.inject.Singleton import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.asContextElement -import java.util.UUID import java.util.concurrent.Callable import kotlin.coroutines.CoroutineContext import kotlin.reflect.KFunction @@ -25,7 +24,6 @@ class ActionScope @Inject internal constructor( ) : AutoCloseable { companion object { private val threadLocalInstance = ThreadLocal() - private val threadLocalUUID = ThreadLocal() } /** @@ -119,22 +117,11 @@ class ActionScope @Inject internal constructor( threadLocalInstance.set(instance) - // If an action scope had previously been entered on the thread, re-use its UUID. - // Otherwise, generate a new one. - if (threadLocalUUID.get() == null) { - threadLocalUUID.set(UUID.randomUUID()) - } - return this } override fun close() { threadLocalInstance.remove() - - // Explicitly NOT removing threadLocalUUID because we want to retain the thread's UUID if - // the action scope is re-entered on the same thread. - // The only way in which threadLocalUUID is removed is through garbage collection, which occurs - // when the thread is no longer alive. } /** Returns true if currently in the scope */ @@ -148,12 +135,11 @@ class ActionScope @Inject internal constructor( check(inScope()) { "not running within an ActionScope" } val currentInstance = threadLocalInstance.get() - val currentThreadUUID = threadLocalUUID.get() return Callable { - // If the original thread is the same as the thread that calls the Callable and we are already + // If the original scope is the same as the scope that calls the KFunction and we are already // in scope, then there is no need to re-enter the scope. - if (inScope() && currentThreadUUID == threadLocalUUID.get()) { + if (inScope() && currentInstance === threadLocalInstance.get()) { c.call() } else { currentInstance.inScope { @@ -171,8 +157,7 @@ class ActionScope @Inject internal constructor( check(inScope()) { "not running within an ActionScope" } val currentInstance = threadLocalInstance.get() - val currentThreadUUID = threadLocalUUID.get() - return WrappedKFunction(currentInstance, this, f, currentThreadUUID) + return WrappedKFunction(currentInstance, this, f) } /** @@ -183,12 +168,11 @@ class ActionScope @Inject internal constructor( check(inScope()) { "not running within an ActionScope" } val currentInstance = threadLocalInstance.get() - val currentThreadUUID = threadLocalUUID.get() return { - // If the original thread is the same as the thread that calls the KFunction and we are already + // If the original scope is the same as the scope that calls the KFunction and we are already // in scope, then there is no need to re-enter the scope. - if (inScope() && currentThreadUUID == threadLocalUUID.get()) { + if (inScope() && currentInstance === threadLocalInstance.get()) { f.invoke() } else { currentInstance.inScope(f) @@ -238,12 +222,11 @@ class ActionScope @Inject internal constructor( val instance: Instance, val scope: ActionScope, val wrapped: KFunction, - val threadUUID: UUID ) : KFunction { override fun call(vararg args: Any?): T { - // If the original thread is the same as the thread that calls the KFunction and we are already + // If the original scope is the same as the scope that calls the KFunction and we are already // in scope, then there is no need to re-enter the scope. - return if (scope.inScope() && threadUUID == threadLocalUUID.get()) { + return if (scope.inScope() && instance === threadLocalInstance.get()) { wrapped.call(*args) } else { instance.inScope { @@ -253,9 +236,9 @@ class ActionScope @Inject internal constructor( } override fun callBy(args: Map): T { - // If the original thread is the same as the thread that calls the KFunction and we are already + // If the original scope is the same as the scope that calls the KFunction and we are already // in scope, then there is no need to re-enter the scope. - return if (scope.inScope() && threadUUID == threadLocalUUID.get()) { + return if (scope.inScope() && instance === threadLocalInstance.get()) { wrapped.callBy(args) } else { instance.inScope { diff --git a/misk-action-scopes/src/test/kotlin/misk/scope/ActionScopePropagationTest.kt b/misk-action-scopes/src/test/kotlin/misk/scope/ActionScopePropagationTest.kt index 03536548556..5088909357a 100644 --- a/misk-action-scopes/src/test/kotlin/misk/scope/ActionScopePropagationTest.kt +++ b/misk-action-scopes/src/test/kotlin/misk/scope/ActionScopePropagationTest.kt @@ -34,11 +34,13 @@ internal class ActionScopePropagationTest { keyOf(Names.named("from-seed")) to "my seed data" ) - val callable = scope.enter(seedData).use { + val actionScopeInstance = scope.create(seedData) + + val callable = actionScopeInstance.inScope { scope.propagate(Callable { tester.fooValue() }) } - scope.enter(seedData).use { + actionScopeInstance.inScope { // Submit to same thread after we've already entered the scope val result = directExecutor.submit(callable).get() assertThat(result).isEqualTo("my seed data and bar and foo!") @@ -55,7 +57,9 @@ internal class ActionScopePropagationTest { keyOf(Names.named("from-seed")) to "my seed data" ) - val callable = scope.enter(seedData).use { + val actionScopeInstance = scope.create(seedData) + + val callable = actionScopeInstance.inScope { scope.propagate(Callable { tester.fooValue() }) } @@ -74,13 +78,15 @@ internal class ActionScopePropagationTest { keyOf(Names.named("from-seed")) to "my seed data" ) + val actionScopeInstance = scope.create(seedData) + // Propagate on the the KCallable directly val f: KFunction = tester::fooValue - val callable = scope.enter(seedData).use { + val callable = actionScopeInstance.inScope { scope.propagate(f) } - scope.enter(seedData).use { + scope.enter(actionScopeInstance).use { // Submit to same thread after we've already entered the scope val result = directExecutor.submit( Callable { @@ -126,12 +132,14 @@ internal class ActionScopePropagationTest { keyOf(Names.named("from-seed")) to "my seed data" ) + val actionScopeInstance = scope.create(seedData) + // Propagate on a lambda directly - val function = scope.enter(seedData).use { + val function = actionScopeInstance.inScope { scope.propagate { tester.fooValue() } } - scope.enter(seedData).use { + actionScopeInstance.inScope { // Submit to same thread after we've already entered the scope val result = directExecutor.submit(Callable { function() }).get() assertThat(result).isEqualTo("my seed data and bar and foo!")