Context
Follow-up from the code review of #1368 (the Channels config-TUI async-write migration).
Two findings were consciously deferred there because their proper fix is architectural, not a
hotfix. The disposal crash-vector and runtime cancellation were fixed in da385500; this issue
tracks the remaining live-editing race and the shared-seam altitude recommendation.
Problem: off-loop view-model mutation races the render loop
The Termina render loop runs on a thread-pool thread with no SynchronizationContext, so await
continuations resume on arbitrary thread-pool threads — not back on the loop. In
ChannelsConfigViewModel, two paths mutate shared view-model state from those off-loop continuations,
concurrently with the loop thread reading/editing the same state for rendering and input handling:
- Background label refresh —
RefreshChannelLabelsAsync → ReconcileResolvedChannels →
SetChannelIds / RemapChannelAudiences / WriteChannelConfigToDisk
(src/Netclaw.Cli/Tui/Config/ChannelsConfigViewModel.cs).
- Save + reload tail —
SaveAsync's _mapper.Load + Step.OnEnter + _mapper.ApplyToStep,
which now runs off-loop when an autosave awaits an in-flight refresh.
Consequences
- A concurrent on-loop edit during a network resolve — e.g.
←/→ to change a channel's audience
(ACL trust tier) or Backspace to remove it — can be silently lost or land on a channel that
reconcile concurrently remapped/dropped. Because the trust tier is a security-relevant value, a lost
lowering edit fails open.
_channelAudiences is a plain Dictionary<ChannelType, Dictionary<string, TrustAudience>> mutated
from two threads with no synchronization → torn read / lost update / "collection modified".
The window is narrow and self-limiting (the refresh only mutates while channels are non-canonical,
i.e. right after adding a name-based channel), and largely pre-existing — the background refresh
has mutated off-loop since the label-normalization feature shipped. #1368 widened it by moving the
save+reload tail off-loop too.
Why it was deferred
Termina exposes only RequestRedraw() as a thread-safe hop onto the loop — it triggers a re-render but
cannot run arbitrary code on the loop thread. So there is no clean way to keep these mutations on the
loop today. Fixing it properly is a real trade-off, e.g. one of:
- Add a Termina primitive to post an
Action onto the loop's event channel (then marshal all
reconcile/reload state writes through it).
- Route the mutating handlers (audience / remove / toggle / DM / users) through the async write chain
with a cancel-and-await of the in-flight refresh before mutating — eliminates the concurrent
writer at the cost of slight per-edit latency.
- Make the shared channel state thread-safe (lock or atomic snapshot-and-swap of
_channelAudiences)
— prevents corruption but not lost updates.
Related altitude item (review #10)
The per-VM serialization chain (EnqueueConfigWriteAsync / _pendingConfigWrite) added for Channels is
bespoke. Search needed its own re-entrancy guard (da385500); future async config VMs will rediscover
the deadlock + race the hard way. Consider hoisting the off-loop dispatch + write-ordering into a shared
seam (ConfigAutosave, or an async-aware Termina key dispatch that awaits and surfaces faults) so every
config VM gets both by construction rather than by per-VM discipline.
References
Context
Follow-up from the code review of #1368 (the Channels config-TUI async-write migration).
Two findings were consciously deferred there because their proper fix is architectural, not a
hotfix. The disposal crash-vector and runtime cancellation were fixed in
da385500; this issuetracks the remaining live-editing race and the shared-seam altitude recommendation.
Problem: off-loop view-model mutation races the render loop
The Termina render loop runs on a thread-pool thread with no
SynchronizationContext, soawaitcontinuations resume on arbitrary thread-pool threads — not back on the loop. In
ChannelsConfigViewModel, two paths mutate shared view-model state from those off-loop continuations,concurrently with the loop thread reading/editing the same state for rendering and input handling:
RefreshChannelLabelsAsync→ReconcileResolvedChannels→SetChannelIds/RemapChannelAudiences/WriteChannelConfigToDisk(
src/Netclaw.Cli/Tui/Config/ChannelsConfigViewModel.cs).SaveAsync's_mapper.Load+Step.OnEnter+_mapper.ApplyToStep,which now runs off-loop when an autosave awaits an in-flight refresh.
Consequences
←/→to change a channel's audience(ACL trust tier) or
Backspaceto remove it — can be silently lost or land on a channel thatreconcile concurrently remapped/dropped. Because the trust tier is a security-relevant value, a lost
lowering edit fails open.
_channelAudiencesis a plainDictionary<ChannelType, Dictionary<string, TrustAudience>>mutatedfrom two threads with no synchronization → torn read / lost update / "collection modified".
The window is narrow and self-limiting (the refresh only mutates while channels are non-canonical,
i.e. right after adding a name-based channel), and largely pre-existing — the background refresh
has mutated off-loop since the label-normalization feature shipped. #1368 widened it by moving the
save+reload tail off-loop too.
Why it was deferred
Termina exposes only
RequestRedraw()as a thread-safe hop onto the loop — it triggers a re-render butcannot run arbitrary code on the loop thread. So there is no clean way to keep these mutations on the
loop today. Fixing it properly is a real trade-off, e.g. one of:
Actiononto the loop's event channel (then marshal allreconcile/reload state writes through it).
with a cancel-and-await of the in-flight refresh before mutating — eliminates the concurrent
writer at the cost of slight per-edit latency.
_channelAudiences)— prevents corruption but not lost updates.
Related altitude item (review #10)
The per-VM serialization chain (
EnqueueConfigWriteAsync/_pendingConfigWrite) added for Channels isbespoke.
Searchneeded its own re-entrancy guard (da385500); future async config VMs will rediscoverthe deadlock + race the hard way. Consider hoisting the off-loop dispatch + write-ordering into a shared
seam (
ConfigAutosave, or an async-aware Termina key dispatch that awaits and surfaces faults) so everyconfig VM gets both by construction rather than by per-VM discipline.
References
b1fe1194(async migration),da385500(lifecycle hardening).