Fix circuit lifetime, lock ordering and cross-thread stat races in threaded UDP receive

[Copilot]

Description

Follow-up to review feedback on the threaded UDP receive path (findings 1, 5, 6, 7, 8, 9). The receiver thread was using a raw LLCircuitData* that the main thread could delete mid-decode, acquired mCircuitMutex/mDataMutex in inconsistent orders, and mutated message-template counters, decode-timing stats and packet-drop simulation state without synchronization.

Circuit lifetime (1)

• LLCircuitData derives from std::enable_shared_from_this; LLCircuit::circuit_data_map holds std::shared_ptr.
• New LLCircuit::findCircuitRef() returns a pinned reference. decodeDataOwned() uses it, so the circuit stays alive across validation, ACK bookkeeping, duplicate handling and accounting. findCircuit() remains as a raw-pointer wrapper for main-thread callers.
• removeCircuitData() erases under mCircuitMutex, parks still-referenced circuits in mRetiredCircuits, and destroys circuits only after releasing the mutex; purgeRetiredCircuits() reclaims them. updateWatchDogTimers(), resendUnackedPackets() and isCircuitAlive() were converted to match.

// message.cpp — pins the circuit for the whole decode
LLCircuit::circuit_data_ptr cdp = mCircuitInfo.findCircuitRef(host);

Lock ordering (5)

• Documented one global order in llcircuit.h: LLCircuit::mCircuitMutex → LLCircuitData::mDataMutex, never inverted, and no callbacks or sends under either lock.
• collectRAck() was inverted and now takes mCircuitMutex first.
• sendAcks() swaps each circuit's pending ACKs out while both locks are held (so acks collected afterwards re-register the circuit), then sends with no lock held, holding a shared_ptr to keep the circuit alive.

Template counters and timing stats (6, 7)

• mReceiveCount, mReceiveBytes, mReceiveInvalid, mTotalDecoded, mTotalDecodeTime, mMaxDecodeTimePerMsg are now atomic and private, reached through recordReceive(), recordReceiveCount(), resetReceiveCounts(), recordDecodeTime(), resetDecodeStats() and getters. Float add/max use CAS loops.
• LLMessageTemplate gets explicit copy ctor/assignment, since atomics delete the implicit ones and existing tests copy templates.

Packet-drop simulation (8)

• mDropPercentage and mPacketsToDrop are atomic; computeDrop() consumes a pending drop via CAS. A percentage hit still does not consume a requested drop (the old code incremented then immediately decremented the counter).

Invalid-circuit logging (9)

• logMsgFromInvalidCircuit(host, recv_reliable) instead of passing pkt.getPacketIDChecked(); other logging call sites were checked for the correct reliability value.

Review items 2, 3, 4 and 10 are untouched.

Related Issues

• Please link to a relevant GitHub issue for additional context.
  • Bug Fix: Link to an issue that includes reproduction steps and testing guidance.
  • Feature/Enhancement: Link to an issue with a write-up, rationale, and requirements.

---

Checklist

Please ensure the following before requesting review:

• I have provided a clear title and detailed description for this pull request.
• If useful, I have included media such as screenshots and video to show off my changes.
• The PR is linked to a relevant issue with sufficient context.
• I have tested the changes locally and verified they work as intended.
• All new and existing tests pass.
• Code follows the project's style guidelines.
• Documentation has been updated if needed.
• Any dependent changes have been merged and published in downstream modules
• I have reviewed the contributing guidelines.

---

Additional Notes

Adds indra/llmessage/tests/llmessagetemplate_test.cpp (registered in CMakeLists.txt) covering counter/timing accumulation and reset, concurrent updates from four threads, and the new explicit copy operations.

Verification was limited to g++ -fsyntax-only on the touched llmessage translation units and a standalone harness exercising the atomic add/max helpers and computeDrop() semantics under contention — a full autobuild and the tut integration tests could not be run in this environment, so the new test still needs to be executed in CI.

[akleshchev]

@copilot these LLCircuit changes look excessive for the purpose of removal alone. Is there a simpler solution? Perhaps there can be some LLCircuir graveyard that gets cleaned by the UDP thread in some fassion? removeCircuitData appears to be strictly main thread, so it's only UDP that has to worry about circuit dying.

[Copilot]

@copilot these LLCircuit changes look excessive for the purpose of removal alone. Is there a simpler solution? Perhaps there can be some LLCircuir graveyard that gets cleaned by the UDP thread in some...

Switched to the graveyard approach in 6d4b4b4 — LLCircuitData is back to plain new/delete ownership and raw pointers everywhere (no shared_ptr, no enable_shared_from_this, findCircuitRef() is gone).

How it works now:

• removeCircuitData() (main thread) erases the circuit from the maps as before, but while the receiver thread is alive it pushes it onto mGraveyard together with the current value of mCircuitsUnusedCount instead of deleting it.
• The UDP thread calls markCircuitsUnused() once per run() iteration, at the top, where it holds no circuit pointer. That just bumps the atomic counter.
• cleanupGraveyard() (main thread, called from removeCircuitData() and once per updateWatchDogTimers()) deletes every entry whose stored counter differs from the current one: a lookup that succeeded must have happened before the erase, and that use ends before the thread's next bump, so a changed counter means nobody can be holding the pointer. Deletion stays on the main thread because ~LLCircuitData() fires reliable-packet callbacks.
• setDeferredDeletion() is flipped on when the receiver thread is created and off after it is joined, so without the thread (tests, shutdown) circuits are deleted immediately and the graveyard is flushed.

The only other change left in removeCircuitData() is deleting the circuit after releasing mCircuitMutex, since the destructor runs callbacks that can call back into the circuit maps.