[WIP] Discussion: Incrementally improving Server TLS cleanup and lifecycle boundaries

[darion-yaphet]

#3590 originally addressed resource cleanup after a failed Server startup. Further discussion revealed that some bthreads may still hold KeyTables borrowed from the Server pool when cleanup begins. More detailed design plans will be discussed here. @chenBright

Deleting the TLS key immediately may cause late-returned data to skip destruction. Deferring key deletion preserves the destructor entry point, but does not ensure that the non-owned factory or destructor arguments remain valid.

I suggest addressing small, well-defined fixes first, then discussing the lifecycle contract separately, so that a local cleanup fix does not expand into a redesign of the entire bthread runtime.

Overall approach and proposed steps

The main idea is to separate three events that currently overlap during cleanup:

1. Closing the pool cache: stop borrowing and caching KeyTables, then destroy the cached entries.
2. Closing factory access: prevent new factory calls and wait for calls already admitted to finish.
3. Retiring the TLS key and callback context: remove the destructor entry point and release its arguments only when no execution path can still access them.

These events have different completion conditions. Closing the pool does not mean every borrowed KeyTable has returned. Keeping the key valid does not keep the factory alive. A zero factory-call count does not prove that no thread has already copied the callback arguments.

The proposed work should therefore proceed in small, separately reviewable steps.

---

Step 1: Fix the confirmed concurrency issues independently

Synchronize pool pointer access and move user destructors outside pool and cache locks, without changing task ownership or existing interfaces.

Step 2: Agree on the cleanup contract

Define what cleanup completion guarantees: whether tasks may retain TLS data, when the factory may be released, and how late TLS access should behave.

Step 3: Resolve callback-context lifetime

Ensure late calls can safely access the required context, and define when the key and callback arguments can be released.

Step 4: Implement the agreed cleanup order

For the gate-based option, destroy cached data before closing the gate, wait for admitted calls, then safely retire the key and context. Evaluate broader ownership changes separately.

Step 5: Validate each stage

Add targeted correctness tests and repeatable performance benchmarks. Verify access safety and complete data reclamation as separate guarantees.

[darion-yaphet]

1. Small fixes that can proceed independently

Synchronize access to pool pointers

borrow_keytable() reads pool->list and pool->free_keytables before acquiring pool->rwlock, while the destruction path modifies these fields. These concurrent accesses are not synchronized.

The checks could be moved under the lock, or the relevant reads and writes could consistently use atomic operations. If unlocked reads remain as hints, the state must still be checked again under the lock.

Making only the reads atomic while retaining concurrent non-atomic writes would not eliminate the data race. An atomic pointer also does not, by itself, keep the pointed-to object alive.

Move user destructors outside cache management locks

This must cover both the pool write lock and ThreadLocal::_mutex. The ThreadLocal destructor deletes per-thread cache objects while holding its internal mutex, so releasing only the outer pool lock is insufficient.

We could reuse ThreadLocal::for_each(): after closing the pool cache and excluding concurrent list access, detach the nodes from each KeyTableList into a local list during iteration. Destroy the KeyTables after releasing the relevant locks, then delete the empty containers.

The for_each() mutex protects the collection of objects; it does not replace pool-level synchronization for list access. Its callback must not invoke user destructors.

These two changes can be validated independently without waiting for a decision on task ownership or factory ownership.