Possible bug with `ThreadLocal.asContextElement`

[alamothe]

Hello,
I'm observing a strange behavior with the following code:

	val t: ThreadLocal<String> = ThreadLocal.withInitial { "a" }

	@Test
	fun testContext1() = runBlocking {
		println(t.get())
		withContext(coroutineContext.plus(t.asContextElement("b"))) {
			println(t.get())
		}
		println(t.get())
	}

	@Test
	fun testContext2() = runBlocking {
		println(t.get())
		withContext(coroutineContext.plus(t.asContextElement("b"))) {
			println(t.get())
			delay(Random.nextLong(100))
			println(t.get())
		}
		println(t.get())
	}

	@Test
	fun testContext3() = runBlocking {
		println(t.get())
		async(coroutineContext.plus(t.asContextElement("b"))) {
			println(t.get())
			delay(Random.nextLong(100))
			println(t.get())
		}.await()
		println(t.get())
	}

The result is:

• testContext1 prints aba - expected
• testContext2 prints abbb - It looks like the value is not restored for some reason. Is this a bug? What's the explanation?
• testContext3 prints abba - expected

Please help!

I'm using org.jetbrains.kotlinx:kotlinx-coroutines-core:1.1.1

[elizarov]

It is not exactly a bug, but a side-effect of the way asContextElement is implemented. See, your main coroutine (in runBlocking) does not have t's context element, so this is interpreted as "main coroutine do not care about this thread local". So then, depending on specific ways you get back to the main tread, the value of t can be either restored or not restored for the main coroutine.

If you write runBlocking(t.asContextElement("a")) { ... } in all your tests, then they all are going to run as expected.

Also note, then when you use withContext function you don't have to write withContext(coroutineContext.plus(other)) { ... }, because that merging of current context with the other context is exactly what withContext does (see its docs), so you can simply write withContext(other) { ... }.

Does it help?

[alamothe]

Hey @elizarov thank you for your reply & the tip for withContext.

I changed the test like this:

	@Test
	fun testContext2() {
		println(t.get())
		runBlocking {
			println(t.get())
			withContext(t.asContextElement("b")) {
				println(t.get())
				delay(Random.nextLong(100))
				println(t.get())
			}
			println(t.get())
		}
		println(t.get())
	}

It prints a a b b b a.

Are you basically saying that, between the outermost asContextElement and the end of the coroutine, the value of ThreadLocal is undefined? I find that very troubling. In my opinion, such undefined behavior could lead to disastrous bugs.

How about after the coroutine? Is it guaranteed to be restored?

[alamothe]

Consider also this case where the value depends on previous value (not uncommon):

	val tInt: ThreadLocal<Int> = ThreadLocal.withInitial { 0 }

	@Test
	fun testContext4() {
		println(tInt.get())
		runBlocking {
			println(tInt.get())
			withContext(tInt.asContextElement(tInt.get() + 1)) {
				println(tInt.get())
				// Comment for other behavior
				delay(Random.nextLong(100))
			}
			println(tInt.get())
			withContext(tInt.asContextElement(tInt.get() + 1)) {
				println(tInt.get())
				// Comment for other behavior
				delay(Random.nextLong(100))
			}
			println(tInt.get())
		}
		println(tInt.get())
	}

So now in the second withContext, the value could be both 1 or 2. In practice, withContext can be deep in the stack and have no idea if it is outermost asContextElement or not.

[alamothe]

If I understand correctly, the only workaround is to remember to set as a context element every time a coroutine is started. But what if that's happening at 100 places (not uncommon, consider the codebase which is migrating to coroutines)

[elizarov]

The design principle of t.asContextElement(value) is this -- a coroutine with this element in its context will have value set in the corresponding thread local. Indeed, there are no guarantees for coroutines that do have this element in their context, as they are not supposed to ever care about it. If they do care about this thread-local, they should have it in their context.

You don't need to remember to always set it if you use the structured concurrency approach and always launch your coroutines as children of other coroutines. Once you set the corresponding element in the context of your top-level scope, it is going to be inherited by all your coroutines.

[alamothe]

You don't need to remember to always set it if you use the structured concurrency approach and always launch your coroutines as children of other coroutines. Once you set the corresponding element in the context of your top-level scope, it is going to be inherited by all your coroutines.

What I was referring to is usage in a servlet container. In this case, each request will start outside of a coroutine (as a separate thread), and then a coroutine will be started at some point via runBlocking. Code outside of a coroutine will care about a thread-local value. To be able to access it both inside and outside of a coroutine - isn't that precisely the idea behind ThreadLocal.asContextElement?

So in this scenario, does the implementation guarantee that the value will be restored? In my example, outside of runBlocking? (my tests do show the value being restored after runBlocking)

I hope the answer is yes. But even so, you should realize the inconsistency here:

1. Value of a thread-local is well-defined inside a coroutine which has it in its context.
2. Value of a thread-local is well-defined outside of coroutine.
3. Value of a thread-local is undefined inside a coroutine which doesn't have it in its context.

Isn't it well reasoned to expect 2 and 3 to act the same? Outside of a coroutine there is no context at all. It's troublesome that the behavior is different. I am sure that someone will get bitten by this.

[elizarov]

@alamothe The value of thread-local is guaranteed to be restored when you go outside of the scope of any of the coroutines. If you write the following code:

// not in a coroutine here
runBlocking(t.asContextElement()) {

}
// the value of t is guaranteed to be restored here

[alamothe]

Got it. The way I solved it is to explicitly add to context every time I start a coroutine like that, and even implement as a helper function.

I still consider this behavior unintuitive, and suggest adding a warning to the docs, stating that "the value of a thread-local is undefined inside a coroutine which doesn't have it in its context", with some examples.