Bump to v1.6.0: peer-address recovery, reconnect crash, long-run memory/CPU leak
Three reliability fixes. Wire format and audio pipeline unchanged from v1.4 / v1.5 — all interoperate. Peer address recovery: * When a tracked peer goes Unreachable (its resolved address — often a stale DNS / Pi-hole record, or a peer that rebooted onto a new IP) but the same peer is still heartbeat-pinging us from a different address, RemSound now adopts the live address instead of transmitting to a dead one. HeartbeatService records untracked ping sources; MainForm's TryAdoptLiveHeartbeatAddress (1 Hz) re-points the sender, heartbeat tracking and receiver allow-list. Conservative: fires only on the unambiguous one-unreachable-and-one-source case, private-range (RFC1918) addresses only so a relay can't hijack the sender, 10 s cooldown. Reconnect crash: * Fixed IndexOutOfRangeException in MainForm.SyncConnectedList. A churny peer-list rebuild (peer reboot) left SelectedIndex pointing past the rebuilt item array; the 1 Hz status timer read SelectedItem and crashed the app. New SafeSelectedItem bounds-checks the index; applied to all three timer-driven sync methods. The status tick is also wrapped in try/catch so a transient WinForms hiccup logs instead of crashing. Long-run memory / CPU leak: * A receiver left running for hours grew to gigabytes and climbing CPU. Decoder sessions orphaned by peer reconnects were not reaped — every reconnect mints a fresh (endpoint, streamId) key, and PruneIdleSessions silently skipped sessions whose PlayoutEngine lookup missed. Rewrote it to reap on each session's own LastWriteUtc (no cross-dictionary lookup), added a hard MaxLiveSessions cap as a backstop, and a "stream sessions live: N" diagnostic line. Bounds both memory and render-thread CPU. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.7
parent
9ceff8bcc1
commit
918ca6cac0
@@ -20,6 +20,31 @@ internal sealed class AboutDialog : Form
|
||||
/// updates" path.</summary>
|
||||
private const string ReleaseNotes =
|
||||
"""
|
||||
RemSound v1.6
|
||||
|
||||
Three reliability fixes. No wire-format or audio-pipeline
|
||||
changes — v1.5 and v1.6 peers interoperate.
|
||||
|
||||
Bug fixes:
|
||||
* Peer address recovery. If the address you connected
|
||||
to goes unreachable — a peer rebooted onto a new IP,
|
||||
or a computer name resolved to a stale address —
|
||||
RemSound now follows the peer to the live address it
|
||||
is heartbeating from, instead of sending audio into
|
||||
the void. Recovers on its own within a few seconds.
|
||||
Limited to private-network addresses so a relay can
|
||||
never be mistaken for a moved peer.
|
||||
* Fixed a crash that could happen when a peer
|
||||
reconnected (e.g. after rebooting). The Connectivity
|
||||
peer list could be read mid-rebuild with a stale
|
||||
index and bring the app down from the status timer.
|
||||
* Fixed runaway memory and CPU on a long-running
|
||||
receiver. Decoder sessions orphaned by peer
|
||||
reconnects were not being reclaimed — over hours they
|
||||
piled up, each holding a multi-megabyte buffer and
|
||||
costing render-thread time every callback. They are
|
||||
now reaped once idle, with a hard cap as a backstop.
|
||||
|
||||
RemSound v1.5
|
||||
|
||||
Menu reorganisation, multi-peer audio-routing fix, recording
|
||||
|
||||
+104
-10
@@ -293,6 +293,9 @@ public sealed class MainForm : Form
|
||||
private int continuousTuneIntervalSec = 5;
|
||||
private long lastObservedUnderrunCount;
|
||||
private HeartbeatService? heartbeatService;
|
||||
// Last time TryAdoptLiveHeartbeatAddress re-pointed the sender at a peer's live address.
|
||||
// Gives a fresh endpoint time to prove healthy before another swap can fire (anti-thrash).
|
||||
private DateTime lastAddressAdoptionUtc = DateTime.MinValue;
|
||||
// Tracks the most recent PeerHealthState we observed for each peer endpoint, so we can
|
||||
// detect transitions and play the appropriate cue. Connect: any state → Healthy.
|
||||
// Disconnect: any state → Unreachable. Stale doesn't fire (it's a transient).
|
||||
@@ -866,13 +869,25 @@ public sealed class MainForm : Form
|
||||
// --- Status / health ticker ---
|
||||
statusTimer.Tick += (_, _) =>
|
||||
{
|
||||
UpdateStatus();
|
||||
SnapshotLogIfDue();
|
||||
EnsureRequestedAudioRunning();
|
||||
// Refresh the Connectivity tab's peer lists from the same 1 Hz tick — replaces
|
||||
// the dialog's old 1.5 s dedicated refresh timer. Each Sync* helper short-circuits
|
||||
// when its signature is unchanged so NVDA isn't spammed with re-announcements.
|
||||
SyncAllPeerLists();
|
||||
// Belt-and-braces: this is a 1 Hz UI tick — a transient WinForms hiccup (e.g. a
|
||||
// stale-index ItemArray throw during a churny peer-list rebuild) must never take
|
||||
// the whole app down with a crash dialog. Log and ride it out; the next tick
|
||||
// recovers. The individual Sync* methods are also hardened (see SafeSelectedItem).
|
||||
try
|
||||
{
|
||||
UpdateStatus();
|
||||
SnapshotLogIfDue();
|
||||
EnsureRequestedAudioRunning();
|
||||
TryAdoptLiveHeartbeatAddress();
|
||||
// Refresh the Connectivity tab's peer lists from the same 1 Hz tick — replaces
|
||||
// the dialog's old 1.5 s dedicated refresh timer. Each Sync* helper short-circuits
|
||||
// when its signature is unchanged so NVDA isn't spammed with re-announcements.
|
||||
SyncAllPeerLists();
|
||||
}
|
||||
catch (Exception ex)
|
||||
{
|
||||
AppendLogEntry($"status tick: {ex.GetType().Name}: {ex.Message}");
|
||||
}
|
||||
};
|
||||
|
||||
// --- Hot-swap device watcher ---
|
||||
@@ -2307,6 +2322,20 @@ public sealed class MainForm : Form
|
||||
return $"{(int)span.TotalHours} hour{((int)span.TotalHours == 1 ? "" : "s")} {span.Minutes} minute{(span.Minutes == 1 ? "" : "s")}";
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Reads <c>list.SelectedItem</c> without the IndexOutOfRangeException WinForms' internal
|
||||
/// ItemArray throws when <c>SelectedIndex</c> is briefly left pointing past the item array.
|
||||
/// That happens during churny peer-list rebuilds (peer reboots, rapid reconnects): the
|
||||
/// 1 Hz Sync* tick read <c>SelectedItem</c> — whose getter blindly does Items[SelectedIndex]
|
||||
/// — and crashed the whole app from a timer callback. Bounds-check the index ourselves,
|
||||
/// the same defensive pattern the ItemCheck handlers already use. 2026-05-15.
|
||||
/// </summary>
|
||||
private static object? SafeSelectedItem(ListBox list)
|
||||
{
|
||||
var i = list.SelectedIndex;
|
||||
return i >= 0 && i < list.Items.Count ? list.Items[i] : null;
|
||||
}
|
||||
|
||||
private void SyncConnectedList()
|
||||
{
|
||||
var desired = new List<(PeerListItem Item, Guid Id)>();
|
||||
@@ -2332,7 +2361,7 @@ public sealed class MainForm : Form
|
||||
if (signature != lastConnectedListSignature)
|
||||
{
|
||||
lastConnectedListSignature = signature;
|
||||
var selectedId = connectedPeersList.SelectedItem is PeerListItem si ? si.Peer.InstanceId : Guid.Empty;
|
||||
var selectedId = SafeSelectedItem(connectedPeersList) is PeerListItem si ? si.Peer.InstanceId : Guid.Empty;
|
||||
suppressConnectedCheck = true;
|
||||
try
|
||||
{
|
||||
@@ -2408,7 +2437,7 @@ public sealed class MainForm : Form
|
||||
if (signature == lastDiscoveredListSignature) return;
|
||||
lastDiscoveredListSignature = signature;
|
||||
|
||||
var selectedId = discoveredPeersList.SelectedItem is PeerListItem si ? si.Peer.InstanceId : Guid.Empty;
|
||||
var selectedId = SafeSelectedItem(discoveredPeersList) is PeerListItem si ? si.Peer.InstanceId : Guid.Empty;
|
||||
suppressDiscoveredCheck = true;
|
||||
try
|
||||
{
|
||||
@@ -2444,7 +2473,7 @@ public sealed class MainForm : Form
|
||||
if (signature == lastRememberedListSignature) return;
|
||||
lastRememberedListSignature = signature;
|
||||
|
||||
var selectedEntry = rememberedPeersList.SelectedItem is RememberedPeerItem si ? si.Entry : null;
|
||||
var selectedEntry = SafeSelectedItem(rememberedPeersList) is RememberedPeerItem si ? si.Entry : null;
|
||||
suppressRememberedCheck = true;
|
||||
try
|
||||
{
|
||||
@@ -3321,6 +3350,71 @@ public sealed class MainForm : Form
|
||||
receiver.SetAllowedSenders(SelectedSendEndpoints());
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Stale-address recovery. When exactly one tracked peer has gone Unreachable (its
|
||||
/// resolved address — often a stale DNS answer — has no host behind it) and exactly one
|
||||
/// OTHER address is actively heartbeat-pinging us, that address is almost certainly the
|
||||
/// same peer at its real location. Re-point the audio sender, heartbeat tracking and the
|
||||
/// receiver allow-list at the live address.
|
||||
///
|
||||
/// Deliberately conservative — it fires only on the unambiguous one-unreachable-and-one-
|
||||
/// live case, only for private-range (RFC1918) live addresses (so a relay's public source
|
||||
/// address can never hijack the sender), and with a 10 s cooldown so it can't thrash. The
|
||||
/// messier multi-peer case is left for the user to sort out by hand. Runs once per second
|
||||
/// from the status ticker. 2026-05-15.
|
||||
/// </summary>
|
||||
private void TryAdoptLiveHeartbeatAddress()
|
||||
{
|
||||
if (heartbeatService is null || !connected) return;
|
||||
// Cooldown: adoption re-points the sender; give a freshly-adopted endpoint time to
|
||||
// prove healthy (or fail) before another swap can fire.
|
||||
if (DateTime.UtcNow - lastAddressAdoptionUtc < TimeSpan.FromSeconds(10)) return;
|
||||
|
||||
var unreachable = heartbeatService.GetAllPeerHealth()
|
||||
.Where(h => h.State == PeerHealthState.Unreachable)
|
||||
.ToList();
|
||||
if (unreachable.Count != 1) return; // 0 = nothing wrong; 2+ = ambiguous
|
||||
|
||||
var liveSources = heartbeatService.GetUntrackedPingSources();
|
||||
if (liveSources.Count != 1) return; // 0 = no candidate; 2+ = ambiguous
|
||||
|
||||
var deadEp = unreachable[0].AudioEndpoint;
|
||||
var liveAddr = liveSources[0];
|
||||
if (liveAddr.Equals(deadEp.Address)) return; // same machine — nothing to adopt
|
||||
if (!IsPrivateLanAddress(liveAddr)) return; // never adopt a public / relay source
|
||||
|
||||
// Find the selected-peer entry whose endpoint is the dead one.
|
||||
var match = selectedPeerEndpoints
|
||||
.FirstOrDefault(kv => kv.Value.Address.Equals(deadEp.Address) && kv.Value.Port == deadEp.Port);
|
||||
if (match.Key == Guid.Empty) return;
|
||||
|
||||
// Reuse the dead endpoint's port — a peer that moved on the LAN keeps its audio port.
|
||||
var newEp = new IPEndPoint(liveAddr, deadEp.Port);
|
||||
selectedPeerEndpoints[match.Key] = newEp;
|
||||
var label = selectedPeerLabels.GetValueOrDefault(match.Key, deadEp.Address.ToString());
|
||||
logFile.Event($"heartbeat: adopted live address for \"{label}\": {deadEp} unreachable, peer is pinging from {newEp}");
|
||||
lastAddressAdoptionUtc = DateTime.UtcNow;
|
||||
|
||||
// ApplyAudioRuntime re-points BOTH the audio sender (SetReceivers) and heartbeat
|
||||
// tracking (SetTrackedPeers); PushAllowedReceiveSenders re-points the receiver
|
||||
// allow-list; PushDiscoveryUnicastHints feeds the new address to discovery too.
|
||||
ApplyAudioRuntime();
|
||||
PushAllowedReceiveSenders();
|
||||
PushDiscoveryUnicastHints();
|
||||
}
|
||||
|
||||
/// <summary>True if <paramref name="addr"/> is an IPv4 RFC1918 private-range address
|
||||
/// (10/8, 172.16/12, 192.168/16). Gates stale-address adoption so a relay's public
|
||||
/// source address can never be mistaken for a peer that moved on the LAN.</summary>
|
||||
private static bool IsPrivateLanAddress(IPAddress addr)
|
||||
{
|
||||
if (addr.AddressFamily != AddressFamily.InterNetwork) return false;
|
||||
var b = addr.GetAddressBytes();
|
||||
return b[0] == 10
|
||||
|| (b[0] == 172 && b[1] >= 16 && b[1] <= 31)
|
||||
|| (b[0] == 192 && b[1] == 168);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Wipes the rolling max-gap window and pushes <see cref="lastSourceChangeUtc"/> forward,
|
||||
/// so the next continuous auto-tune tick has nothing to react to. Called whenever a user
|
||||
|
||||
@@ -14,7 +14,7 @@
|
||||
tag_name on the latest GitHub release; bump it on every public release. The
|
||||
AssemblyVersion / FileVersion default to this value, and Assembly.GetName().Version
|
||||
is what the About dialog and the updater both read. -->
|
||||
<Version>1.5.0</Version>
|
||||
<Version>1.6.0</Version>
|
||||
</PropertyGroup>
|
||||
|
||||
<ItemGroup>
|
||||
|
||||
@@ -52,6 +52,13 @@ public sealed class HeartbeatService : IDisposable
|
||||
private readonly object gate = new();
|
||||
private readonly Dictionary<string, PeerState> peers = new(StringComparer.OrdinalIgnoreCase);
|
||||
private readonly Stopwatch monotonic = Stopwatch.StartNew();
|
||||
// Source addresses of recently-received heartbeat Pings, keyed by IP only (the ping's
|
||||
// source port is the peer's ephemeral / NAT port, never the audio port — same reasoning
|
||||
// as the pong IP-only match in HandlePacket). MainForm reads this via
|
||||
// GetUntrackedPingSources to recover from a stale-address situation: a peer we can't
|
||||
// reach at its resolved (e.g. stale-DNS) address but which is pinging us from its real
|
||||
// address. See TryAdoptLiveHeartbeatAddress in MainForm. 2026-05-15.
|
||||
private readonly Dictionary<IPAddress, DateTime> recentPingSources = new();
|
||||
|
||||
private CancellationTokenSource? cts;
|
||||
private Task? sendTask;
|
||||
@@ -153,6 +160,28 @@ public sealed class HeartbeatService : IDisposable
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Addresses that have sent us a heartbeat Ping within the last <see cref="UnreachableWindow"/>
|
||||
/// and are NOT currently tracked peers. Used by MainForm's stale-address recovery: when a
|
||||
/// tracked peer has gone Unreachable but some other address is actively pinging us, that
|
||||
/// address is very likely the same peer at its real location (DNS handed us a stale IP).
|
||||
/// Keyed by IP only — heartbeat ping source ports are ephemeral and carry no peer identity.
|
||||
/// </summary>
|
||||
public IReadOnlyList<IPAddress> GetUntrackedPingSources()
|
||||
{
|
||||
lock (gate)
|
||||
{
|
||||
var cutoff = DateTime.UtcNow - UnreachableWindow;
|
||||
// Prune entries older than the window while we're holding the lock.
|
||||
foreach (var stale in recentPingSources.Where(kv => kv.Value < cutoff).Select(kv => kv.Key).ToList())
|
||||
{
|
||||
recentPingSources.Remove(stale);
|
||||
}
|
||||
var trackedAddrs = peers.Values.Select(p => p.AudioEndpoint.Address).ToHashSet();
|
||||
return recentPingSources.Keys.Where(a => !trackedAddrs.Contains(a)).ToList();
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// One-line summary suitable for the snapshot log column or status label.
|
||||
/// "no peers" / "192.168.1.5: 24ms" / "192.168.1.5: 24ms, 192.168.1.6: unreachable 7s".
|
||||
@@ -279,6 +308,9 @@ public sealed class HeartbeatService : IDisposable
|
||||
if (kind == HeartbeatKind.Ping)
|
||||
{
|
||||
onDiagnostic?.Invoke($"recv ping from={remote}");
|
||||
// Record the source so MainForm can spot a peer that's pinging us from an
|
||||
// address we're not tracking (stale-DNS / DHCP-move recovery).
|
||||
lock (gate) { recentPingSources[remote.Address] = DateTime.UtcNow; }
|
||||
|
||||
// Echo the originator's timestamp back to them as a Pong. Reply target is the
|
||||
// remote source endpoint (whatever socket the ping came in on, that's where to
|
||||
|
||||
@@ -44,6 +44,14 @@ public sealed class AudioReceiver : IDisposable
|
||||
/// though the mix output is unaffected).</summary>
|
||||
public static readonly TimeSpan SessionIdleTimeout = TimeSpan.FromSeconds(4);
|
||||
|
||||
/// <summary>Hard ceiling on concurrently-tracked stream sessions. A backstop for the idle
|
||||
/// prune: even if reconnect churn somehow outpaces the idle sweep, the session table — and
|
||||
/// the multi-MB playout ring each entry owns — can never grow without bound. Far above any
|
||||
/// legitimate scenario (the lobby relay caps peers at 10, and a peer emits at most one
|
||||
/// stream per render lane). When exceeded, <see cref="PruneIdleSessions"/> evicts the
|
||||
/// idlest sessions down to this cap. 2026-05-15.</summary>
|
||||
public const int MaxLiveSessions = 32;
|
||||
|
||||
private readonly Stopwatch uptime = new();
|
||||
private readonly ReceiverDiagnostics diagnostics = new();
|
||||
private readonly PlayoutEngine playoutEngine;
|
||||
@@ -57,6 +65,10 @@ public sealed class AudioReceiver : IDisposable
|
||||
// independent audio mode). For single-lane modes the sender emits one streamId so
|
||||
// the dict still has one entry per peer, identical to the pre-refactor behaviour.
|
||||
private readonly Dictionary<(IPEndPoint Endpoint, ushort StreamId), StreamSession> sessions = new();
|
||||
// Last live-session count emitted to the diagnostic sink. Lets PruneIdleSessions log
|
||||
// only when the count actually changes, so unbounded growth is visible in the log
|
||||
// without spamming it or needing a Task Manager screenshot to notice.
|
||||
private int lastLoggedSessionCount = -1;
|
||||
|
||||
/// <summary>When false (the default), a Format packet arriving with a NEW streamId from
|
||||
/// a peer that already has a session under a DIFFERENT streamId triggers immediate
|
||||
@@ -591,30 +603,54 @@ public sealed class AudioReceiver : IDisposable
|
||||
public void PruneIdleSessions()
|
||||
{
|
||||
var now = DateTime.UtcNow;
|
||||
List<(IPEndPoint Endpoint, ushort StreamId)>? toRemove = null;
|
||||
lock (sessionsLock)
|
||||
{
|
||||
var toRemove = new HashSet<(IPEndPoint Endpoint, ushort StreamId)>();
|
||||
|
||||
// 1) Idle sweep — reap sessions with no decoded write within SessionIdleTimeout.
|
||||
// Reaped on the session's OWN last-write time. The previous implementation
|
||||
// cross-referenced PlayoutEngine.ActiveSessions by (endpoint, streamId) and
|
||||
// silently skipped — leaking the session forever — whenever that lookup missed.
|
||||
// A reconnecting peer never reuses an old key: a sender reboot rerolls the
|
||||
// streamId AND rebinds to a fresh ephemeral source port, so the old session is
|
||||
// always an orphan the lookup-based prune could strand. Reading the session's
|
||||
// own LastWriteUtc removes the lookup, the race, and the leak. 2026-05-15.
|
||||
foreach (var (key, session) in sessions)
|
||||
{
|
||||
// Match SessionPlayout by full key so two streams from the same peer don't
|
||||
// share a single SessionPlayout entry. ActiveSessions iteration is small
|
||||
// (one per active stream).
|
||||
var sp = playoutEngine.ActiveSessions.FirstOrDefault(x =>
|
||||
x.Endpoint.Equals(key.Endpoint) && x.StreamId == key.StreamId);
|
||||
if (sp is null) continue;
|
||||
if (now - sp.LastWriteUtc <= SessionIdleTimeout) continue;
|
||||
toRemove ??= [];
|
||||
toRemove.Add(key);
|
||||
if (now - session.LastWriteUtc > SessionIdleTimeout) toRemove.Add(key);
|
||||
}
|
||||
if (toRemove is not null)
|
||||
|
||||
// 2) Hard-cap backstop. If more than MaxLiveSessions would still remain after the
|
||||
// idle sweep, evict the idlest extras. Guarantees the session table can never
|
||||
// grow without bound whatever churn the idle sweep can't keep up with.
|
||||
var survivors = sessions.Count - toRemove.Count;
|
||||
if (survivors > MaxLiveSessions)
|
||||
{
|
||||
foreach (var key in toRemove)
|
||||
foreach (var key in sessions
|
||||
.Where(kv => !toRemove.Contains(kv.Key))
|
||||
.OrderBy(kv => kv.Value.LastWriteUtc)
|
||||
.Take(survivors - MaxLiveSessions)
|
||||
.Select(kv => kv.Key))
|
||||
{
|
||||
if (sessions.Remove(key, out var session)) session.Dispose();
|
||||
playoutEngine.RemoveSession(key.Endpoint, key.StreamId);
|
||||
diagnosticSink?.Invoke($"stream session pruned (idle): {key.Endpoint} stream={key.StreamId}");
|
||||
toRemove.Add(key);
|
||||
}
|
||||
}
|
||||
|
||||
// 3) Apply removals — drop the StreamSession and its paired SessionPlayout together.
|
||||
foreach (var key in toRemove)
|
||||
{
|
||||
if (sessions.Remove(key, out var session)) session.Dispose();
|
||||
playoutEngine.RemoveSession(key.Endpoint, key.StreamId);
|
||||
diagnosticSink?.Invoke($"stream session pruned: {key.Endpoint} stream={key.StreamId}");
|
||||
}
|
||||
|
||||
// Surface the live-session count whenever it changes, so any accumulation is
|
||||
// visible in the log without summing open/prune events.
|
||||
if (sessions.Count != lastLoggedSessionCount)
|
||||
{
|
||||
lastLoggedSessionCount = sessions.Count;
|
||||
diagnosticSink?.Invoke($"stream sessions live: {sessions.Count}");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -34,6 +34,13 @@ internal sealed class StreamSession : IDisposable
|
||||
public AudioFormatInfo Format { get; }
|
||||
public AudioTransportCodec Codec => (AudioTransportCodec)Format.Codec;
|
||||
|
||||
/// <summary>UTC timestamp of the most recent decoded-audio write into this session's
|
||||
/// playout buffer. <see cref="AudioReceiver.PruneIdleSessions"/> reaps on this directly,
|
||||
/// rather than a cross-dictionary lookup into PlayoutEngine that could miss and strand
|
||||
/// the session forever — a reconnecting peer never reuses its old (endpoint, streamId)
|
||||
/// key, so its previous session is always an orphan that must be reaped by idle age.</summary>
|
||||
public DateTime LastWriteUtc => sessionPlayout.LastWriteUtc;
|
||||
|
||||
/// <summary>For PCM streams: number of incoming packets the assembler rejected outright.</summary>
|
||||
public long PcmFrameRejections => pcmAssembler.RejectionCount;
|
||||
/// <summary>For PCM streams: number of partially-assembled frames discarded mid-flight.</summary>
|
||||
|
||||
Reference in New Issue
Block a user