fix(rns): prevent KeyError on concurrent destination registration (COLUMBA-B8)

[sentry]

This commit addresses a race condition in PythonRnsLxmf.resolveRecipientDestination() that could lead to a KeyError: 'Attempt to register an already registered destination.' from the Python Reticulum library.

Previously, multiple concurrent AIDL calls to dispatchLxmessage targeting the same uncached destination hash could bypass a non-atomic runtime.destinations[hex]?.let check. This allowed multiple threads to concurrently attempt to register the same RNS destination after identity resolution, with the slower thread triggering the KeyError.

The fix replaces the non-atomic check-then-set pattern with runtime.destinations.computeIfAbsent(hex) { ... }. This leverages the ConcurrentHashMap's atomic putIfAbsent semantics, ensuring that only one thread executes the destination creation logic (identity recall, path request, poll loop, and RNS.Destination() construction) for a given hash. Concurrent threads for the same hash will block and receive the already-computed value, thus preventing duplicate registrations and the associated KeyError.

Fixes COLUMBA-B8

[greptile-apps]

Greptile Summary

This PR fixes a real race condition in resolveRecipientDestination where concurrent AIDL threads bypassed the non-atomic ?.let cache check and both tried to register the same RNS destination, causing a KeyError from the Python RNS library. The fix is a one-for-one replacement of the check-then-set pattern with ConcurrentHashMap.computeIfAbsent.

• The duplicate-registration crash is correctly eliminated: computeIfAbsent guarantees the mapping function runs at most once per key for the success path.
• The mapping function contains a Thread.sleep loop that can block for up to 10 seconds (the full PATH_RESOLVE_TIMEOUT_MS). Because ConcurrentHashMap holds a per-bucket lock for the entire duration of the mapping function, any unrelated thread resolving a different destination hash that collides into the same bucket will stall for up to 10 seconds — behaviour the Java documentation explicitly warns against ("computation should be short and simple").
• When identity resolution fails and the lambda throws, no value is stored, so concurrent callers for the same unreachable hash each pay the full 10-second timeout in sequence rather than sharing a single failure.

Confidence Score: 3/5

The crash fix is correct, but the implementation trades one concurrency hazard for another: a 10-second sleep loop inside the ConcurrentHashMap mapping function can serialise unrelated destination lookups that share a bucket.

The fix definitively eliminates the KeyError by making the registration atomic. However, computeIfAbsent holds the bin lock across the full path-resolve poll loop — including up to 40 × 250 ms sleeps — which can stall concurrent lookups for different destinations in the same bucket for the full 10-second window. This is a concrete blocking regression on the hot send path, not a theoretical concern.

PythonRnsLxmf.kt — specifically resolveRecipientDestination, which now blocks the ConcurrentHashMap bin lock for the entire path-resolve timeout.

Important Files Changed

Filename	Overview
rns-backend-py/src/main/kotlin/network/columba/app/rns/backend/py/PythonRnsLxmf.kt	Fixes the duplicate-registration KeyError by using computeIfAbsent, but the mapping function contains a Thread.sleep loop of up to 10 s, holding the ConcurrentHashMap bin lock for that entire window and blocking unrelated destination lookups in the same bucket.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant T1 as Thread A
    participant T2 as Thread B
    participant CHM as ConcurrentHashMap
    participant RNS as Python RNS

    T1->>CHM: computeIfAbsent("hexA")
    Note over CHM: bin lock acquired by Thread A
    T2->>CHM: computeIfAbsent("hexA")
    Note over CHM: Thread B blocks on same bin lock

    T1->>RNS: Identity.recall(hashPy) returns null
    T1->>RNS: Transport.request_path(hashPy)
    loop poll up to 10 s
        T1->>T1: Thread.sleep(250 ms)
        T1->>RNS: Identity.recall(hashPy)
    end

    RNS-->>T1: recipientIdentity resolved
    T1->>RNS: RNS.Destination(...)
    RNS-->>T1: PyObject
    T1->>CHM: store result and release lock
    CHM-->>T1: dest returned

    Note over T2: unblocked, sees existing value
    CHM-->>T2: dest returned without re-computing

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant T1 as Thread A
    participant T2 as Thread B
    participant CHM as ConcurrentHashMap
    participant RNS as Python RNS

    T1->>CHM: computeIfAbsent("hexA")
    Note over CHM: bin lock acquired by Thread A
    T2->>CHM: computeIfAbsent("hexA")
    Note over CHM: Thread B blocks on same bin lock

    T1->>RNS: Identity.recall(hashPy) returns null
    T1->>RNS: Transport.request_path(hashPy)
    loop poll up to 10 s
        T1->>T1: Thread.sleep(250 ms)
        T1->>RNS: Identity.recall(hashPy)
    end

    RNS-->>T1: recipientIdentity resolved
    T1->>RNS: RNS.Destination(...)
    RNS-->>T1: PyObject
    T1->>CHM: store result and release lock
    CHM-->>T1: dest returned

    Note over T2: unblocked, sees existing value
    CHM-->>T2: dest returned without re-computing

Loading

Prompt To Fix All With AI

Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 2
rns-backend-py/src/main/kotlin/network/columba/app/rns/backend/py/PythonRnsLxmf.kt:272-301
**Long-running computation inside `computeIfAbsent` holds the bin lock**

`ConcurrentHashMap.computeIfAbsent` acquires a per-bucket lock for the *entire* duration of the mapping function. The lambda here can call `Thread.sleep(PATH_RESOLVE_POLL_MS)` up to 40 times (10 000 ms / 250 ms), so any unrelated thread resolving a different destination hash that lands in the same bucket will block for up to 10 seconds. The Java documentation explicitly states the function "should be short and simple." With 16 buckets at the default capacity, the collision probability for two concurrent first-time lookups is ~6 %, making this a realistic stall in practice.

A pattern that avoids this is to put a `CompletableFuture<PyObject>` (or Kotlin `CompletableDeferred`) as the map value — the fast `computeIfAbsent` stores the future atomically, and all threads await it outside the bin lock — but that would require a non-trivial refactor. At minimum, the path-poll loop should be extracted and the bin lock not held across the sleep.

### Issue 2 of 2
rns-backend-py/src/main/kotlin/network/columba/app/rns/backend/py/PythonRnsLxmf.kt:287-289
**Sequential timeout accumulation for concurrent callers on the same failing hash**

When identity resolution times out and `RnsException(RnsError.IdentityNotFound(hex))` is thrown from the lambda, `computeIfAbsent` releases the bin lock without storing any value. A second concurrent thread blocked on the same key then runs its own 10-second poll in sequence. With N concurrent senders to an unreachable destination, the last caller can wait up to N × 10 s before surfacing the error — even though they all share the same failure. Consider storing a sentinel or negative-cache entry (with a short TTL) to short-circuit repeat failures.

_{Reviews (1): Last reviewed commit: "fix(rns): prevent KeyError on concurrent..." | Re-trigger Greptile}

[greptile-apps]

Long-running computation inside computeIfAbsent holds the bin lock

ConcurrentHashMap.computeIfAbsent acquires a per-bucket lock for the entire duration of the mapping function. The lambda here can call Thread.sleep(PATH_RESOLVE_POLL_MS) up to 40 times (10 000 ms / 250 ms), so any unrelated thread resolving a different destination hash that lands in the same bucket will block for up to 10 seconds. The Java documentation explicitly states the function "should be short and simple." With 16 buckets at the default capacity, the collision probability for two concurrent first-time lookups is ~6 %, making this a realistic stall in practice.

A pattern that avoids this is to put a CompletableFuture<PyObject> (or Kotlin CompletableDeferred) as the map value — the fast computeIfAbsent stores the future atomically, and all threads await it outside the bin lock — but that would require a non-trivial refactor. At minimum, the path-poll loop should be extracted and the bin lock not held across the sleep.

Prompt To Fix With AI

This is a comment left during a code review.
Path: rns-backend-py/src/main/kotlin/network/columba/app/rns/backend/py/PythonRnsLxmf.kt
Line: 272-301

Comment:
**Long-running computation inside `computeIfAbsent` holds the bin lock**

`ConcurrentHashMap.computeIfAbsent` acquires a per-bucket lock for the *entire* duration of the mapping function. The lambda here can call `Thread.sleep(PATH_RESOLVE_POLL_MS)` up to 40 times (10 000 ms / 250 ms), so any unrelated thread resolving a different destination hash that lands in the same bucket will block for up to 10 seconds. The Java documentation explicitly states the function "should be short and simple." With 16 buckets at the default capacity, the collision probability for two concurrent first-time lookups is ~6 %, making this a realistic stall in practice.

A pattern that avoids this is to put a `CompletableFuture<PyObject>` (or Kotlin `CompletableDeferred`) as the map value — the fast `computeIfAbsent` stores the future atomically, and all threads await it outside the bin lock — but that would require a non-trivial refactor. At minimum, the path-poll loop should be extracted and the bin lock not held across the sleep.

How can I resolve this? If you propose a fix, please make it concise.

[greptile-apps]

Sequential timeout accumulation for concurrent callers on the same failing hash

When identity resolution times out and RnsException(RnsError.IdentityNotFound(hex)) is thrown from the lambda, computeIfAbsent releases the bin lock without storing any value. A second concurrent thread blocked on the same key then runs its own 10-second poll in sequence. With N concurrent senders to an unreachable destination, the last caller can wait up to N × 10 s before surfacing the error — even though they all share the same failure. Consider storing a sentinel or negative-cache entry (with a short TTL) to short-circuit repeat failures.

Prompt To Fix With AI

This is a comment left during a code review.
Path: rns-backend-py/src/main/kotlin/network/columba/app/rns/backend/py/PythonRnsLxmf.kt
Line: 287-289

Comment:
**Sequential timeout accumulation for concurrent callers on the same failing hash**

When identity resolution times out and `RnsException(RnsError.IdentityNotFound(hex))` is thrown from the lambda, `computeIfAbsent` releases the bin lock without storing any value. A second concurrent thread blocked on the same key then runs its own 10-second poll in sequence. With N concurrent senders to an unreachable destination, the last caller can wait up to N × 10 s before surfacing the error — even though they all share the same failure. Consider storing a sentinel or negative-cache entry (with a short TTL) to short-circuit repeat failures.

How can I resolve this? If you propose a fix, please make it concise.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!