[Simn]

When a ThreadLocal is finalised, its slot id goes back on a free list and is handed to the next ThreadLocal. The values that threads stored in that slot are never cleared, though, so a new ThreadLocal can return a value that an old one stored on the same thread. That value can even be of an unrelated type:

import sys.thread.Tls;

function main() {
	var stale = 0;
	for (i in 0...100) {
		var a = new Tls<String>();
		a.value = "left behind";
		a = null;
		cpp.vm.Gc.run(true);
		if (new Tls<String>().value != null) {
			stale++;
		}
	}
	Sys.println('new Tls with a non-null value: $stale of 100'); // 100 of 100
}

This made one of hxcoro's tests fail intermittently. A new TrampolineDispatcher got a Tls slot from a collected thread pool, read that pool's WorkStealingQueue as its Trampoline, and failed with an hxSehException.

The fix gives each slot a generation that's incremented when the slot is reused. Each ThreadLocal remembers the generation it was created with, threads store the generation together with each value, and get returns null when the two don't match. Slot reuse stays as it is, and nothing has to be cleared across threads when a ThreadLocal is finalised. The legacy __hxcpp_tls_get/__hxcpp_tls_set use generation 0, which is what a slot has before its first reuse, so their behaviour doesn't change. getSlot now reads with __get, so reading an unset slot no longer grows the arrays.

With this change, the example above prints 0 of 100, and the hxcoro test passed in 20 of 20 runs instead of failing about one time in four.

🤖 Generated with Claude Code

[Simn]

The example and the description make a lot of sense to me, but I'm not sure if this is really the best way to address it. @Aidan63 Could you take a look?

[Aidan63]

Hmm, yeah, I remember seeing this generation trick somewhere else and I'm not a massive fan. There's also the unrelated unideal situation where even after the TLS is collected the thread will retain the last object stored.

I originally wanted the thread object to not have to worry / store info about TLS objects, but maybe that's not really possible after all. Maybe the thread object could store weak references to TLS objects and that could a basis for eagerly collecting / clearing stale values? I'll have a think.

[Aidan63]

What I think an ideal solution would be is...

Each thread TLS objects (Array<Dynamic>) would become something like Array<WeakRef<Dynamic>>, this way a live thread won't keep objects alive which were stored in a TLS slot after the original TLS object has been collected.
Marking a TLS object is then a special case, it would need to iterate over all the thread objects and mark the object stored in each WeakRef for it's assigned slot to keep them alive. Getting an object from a TLS would need a slight tweak, hx::IsWeakRefValid would need to be used to detect stale / collected objects and return null instead when an id is recycled.

There are two problems with implementing this though, there is a WeakRef class in Immix.cpp but it is not exposed out side of that file but it has zero use within hxcpp, so I'm not sure what the state of it is... You also can't currently access all of the thread objects. Each LocalAllocator is stored for the GC to inspect but thread objects don't have a central storage location. Maybe the local allocator could store each thread object as part of attaching a thread?

Given that the weak ref approach needs a bit more work I'm fine with merging this is as is if you need it for a failing test.

[Simn]

I'm not in a hurry, this was found via HaxeFoundation/hxcoro#164 and I'm fine with holding back that merge until everything has been sorted out properly.

And the more relevant PR for that is #1435 anyway.