Two concurrency bugs identified in PR #2904 review:
1. Replace sync.WaitGroup with sync.Cond-based activeReqCount to avoid
the "WaitGroup is reused before previous Wait has returned" panic that
occurs when Add(1) races with a goroutine-launched Wait().
2. Make panic cleanup conditional: when runTurn panics, only delete the
session's activeTurnStates entry if it still points to our placeholder.
Previously, an unconditional delete could wipe a new message's slot
claimed between the panic and the deferred cleanup.
Unit tests prove an out-of-tree channel type becomes valid and decodable after
RegisterChannelSettings, including the full InitChannelList -> GetDecoded path that
previously failed with "unknown type".
Expose a public registration hook so external packages that register a channel
factory via channels.RegisterFactory can also register the channel's settings
struct prototype. Without this, InitChannelList rejects any channel type absent
from the private channelSettingsFactory map, making out-of-tree channels
impossible without forking. The map access is now guarded by a RWMutex.
This is additive and changes no existing behavior.
Replace silently discarded json.Marshal and json.Unmarshal errors with
explicit checks. If serialization fails, log a warning and either
return early (for the config-level marshal/unmarshal) or skip the
channel (for per-channel marshal). This prevents silent data loss
when channel configuration contains unexpected types.
Replace the Delete+LoadOrStore pattern with CompareAndSwap to
atomically swap in a fresh mutex when a corrupted entry is detected.
Changes:
- Use a retry loop with CAS instead of Delete+LoadOrStore, avoiding
the race where two goroutines could create different mutexes
- Add nil check: a nil *sync.Mutex passes the type assertion but
panics on Lock() — now handled by the CAS recovery path
- Use continue-loop retry instead of unchecked second assertion
CompareAndSwap guarantees that when multiple goroutines detect a
corrupted entry, they converge on the same replacement mutex.
Replace the no-op _ = ok with a warning log when native_search has an
unexpected type. When the type assertion fails, nativeSearch already
defaults to false, which is conservative — but the caller should know
their option was malformed.
The reviewer identified two bugs in the original PR:
1. PATCH /api/config leaves session.dimensions stale: LoadConfig()
derives dimensions from the old dm_scope, and the merge carries
those stale dimensions forward. ApplyDmScope() then exits early
because dimensions is already populated, causing a mismatch between
dm_scope (new) and dimensions (old).
2. Legacy/default configs omit dm_scope in GET response: configs with
explicit dimensions but no dm_scope (including DefaultConfig) return
no dm_scope field, causing the frontend to fall back to its default
('per-channel-peer'), which may not match the actual dimensions.
Fix:
- Add DeriveDmScope() to reverse-map known dimensions arrays to
dm_scope when dm_scope is empty.
- Call it in LoadConfig(), PUT handler, PATCH handler, and
ResetToDefaults() for consistent normalization.
- In PATCH handler, clear stale dimensions from the merge result when
the patch contains session.dm_scope but not session.dimensions,
allowing ApplyDmScope() to re-derive from the new scope.
- Add comprehensive unit tests for DeriveDmScope() and scope
transition scenarios.
