Symptom

Nightly architectural scan flagged HttpClientProvider in all three JVM/Native actual implementations (android, jvm, ios) as having an unsynchronized mutable client field accessed concurrently by ClientNodeManager, NodeObserver, and any other coroutines that call httpClient or sseHttpClient. Under concurrent load, two callers could both observe client == null and each build a new HttpClient, leaking one. rebuild() could close the old client while a concurrent getInstance() was still using it or was mid-check — producing a torn read or returning a closed client.

Root cause

All three HttpClientProvider implementations used a plain private var client: HttpClient? = null with an unsynchronized check-then-set:

1
2
3
4
5
6
7
8
9
10
11
fun getInstance(): HttpClient {
    if (client == null) {   // TOCTOU: two threads can both see null
        client = buildSslClient()
    }
    return client!!
}

fun rebuild() {
    client?.close()         // races with concurrent getInstance()
    client = buildSslClient()
}

rebuild() was also calling close() before assigning the new client, creating a window where getInstance() could return a closed or null client.

Fix

• JVM/Android (HttpClientContainer.jvm.kt, HttpClientContainer.android.kt): Added @Volatile to client and double-checked locking via synchronized(this) in getInstance(). In rebuild(), the new client is built outside the lock, then atomically swapped in; the old client is closed after releasing the lock so callers are never blocked waiting on client construction.
• iOS/Native (HttpClientContainer.ios.kt): Replaced var client: HttpClient? with AtomicReference<HttpClient?>(null). getInstance() uses compareAndSet(null, new) — only one thread wins; the other immediately closes its redundant instance. rebuild() uses getAndSet(new) for an atomic swap, then closes the displaced client.
• Made HttpClientProvider open and buildCioClient()/buildSslClient() internal open to allow test subclasses that override the builder without touching production filesystem paths.
• Added HttpClientProviderConcurrencyTest with three tests: concurrent same-instance guarantee, rebuild produces a new distinct instance, and concurrent rebuild() + getInstance() never returns null.

Prevention

• Any var field in a class whose instance is used from multiple coroutines on Dispatchers.Default or Dispatchers.IO is a data race if not protected. The check is: is the object held in a module-level val? If yes, it outlives any single coroutine and is shared.
• The preferred KMP pattern for lazy initialization shared across threads: @Volatile field + synchronized(this) DCL on JVM; AtomicReference + compareAndSet on Native.
• rebuild()-style operations must build the new resource before acquiring the lock (or before the atomic swap), and close the old resource after releasing the lock. Never close-then-assign — that creates a null window.
• When writing concurrent tests, override the expensive resource builder in a test subclass so the test creates real instances without reading production filesystem paths (/var/lib/krill/trusted, ~/.krill/trusted).