Fix the crash on changing network settings, and apply resynced contacts in batches
ApplyNetworkSettings disposed the station link without clearing it, so ApplyStationLinkSettings disposed it a second time and threw. Only the second function touches the link now, and StationLink.Dispose can be called twice. Contacts from the network are queued and applied together on the window thread. Each one used to rescore the whole log and redraw the log window, which kept the window thread busy for minutes during a resync. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -1,3 +1,4 @@
|
|||||||
|
using System.Collections.Concurrent;
|
||||||
using Avalonia.Threading;
|
using Avalonia.Threading;
|
||||||
using Nonemm.App.Configuration;
|
using Nonemm.App.Configuration;
|
||||||
using Nonemm.Contests;
|
using Nonemm.Contests;
|
||||||
@@ -363,9 +364,7 @@ public sealed class AppSession : IDisposable
|
|||||||
/// multi-operator entry, following what the settings now say.
|
/// multi-operator entry, following what the settings now say.
|
||||||
public void ApplyNetworkSettings()
|
public void ApplyNetworkSettings()
|
||||||
{
|
{
|
||||||
announcing?.Stop();
|
|
||||||
network?.Dispose();
|
network?.Dispose();
|
||||||
link?.Dispose();
|
|
||||||
network = null;
|
network = null;
|
||||||
if (!Settings.NetworkEnabled)
|
if (!Settings.NetworkEnabled)
|
||||||
{
|
{
|
||||||
@@ -378,8 +377,7 @@ public sealed class AppSession : IDisposable
|
|||||||
Settings.NetworkPeers);
|
Settings.NetworkPeers);
|
||||||
// the socket thread must not touch the log: every other change to it is
|
// the socket thread must not touch the log: every other change to it is
|
||||||
// made where the windows read it
|
// made where the windows read it
|
||||||
network.UpdateArrived += (_, update) =>
|
network.UpdateArrived += (_, update) => QueueFromNetwork(update);
|
||||||
Dispatcher.UIThread.Post(() => TakeFromNetwork(update));
|
|
||||||
network.Start();
|
network.Start();
|
||||||
Changed?.Invoke(this, EventArgs.Empty);
|
Changed?.Invoke(this, EventArgs.Empty);
|
||||||
}
|
}
|
||||||
@@ -395,6 +393,7 @@ public sealed class AppSession : IDisposable
|
|||||||
|
|
||||||
public void ApplyStationLinkSettings()
|
public void ApplyStationLinkSettings()
|
||||||
{
|
{
|
||||||
|
announcing?.Stop();
|
||||||
link?.Dispose();
|
link?.Dispose();
|
||||||
link = null;
|
link = null;
|
||||||
if (!Settings.StationLinkEnabled)
|
if (!Settings.StationLinkEnabled)
|
||||||
@@ -412,8 +411,7 @@ public sealed class AppSession : IDisposable
|
|||||||
};
|
};
|
||||||
// the socket thread must not touch the log: every other change to it is
|
// the socket thread must not touch the log: every other change to it is
|
||||||
// made where the windows read it
|
// made where the windows read it
|
||||||
link.UpdateArrived += (_, update) =>
|
link.UpdateArrived += (_, update) => QueueFromNetwork(update);
|
||||||
Dispatcher.UIThread.Post(() => TakeFromNetwork(update));
|
|
||||||
// the resync reads the log rather than being handed it, and another
|
// the resync reads the log rather than being handed it, and another
|
||||||
// station can ask for a rescore when its resync is done
|
// station can ask for a rescore when its resync is done
|
||||||
link.Log = new SessionContactLog(this);
|
link.Log = new SessionContactLog(this);
|
||||||
@@ -607,34 +605,75 @@ public sealed class AppSession : IDisposable
|
|||||||
host.Length > 1 && int.TryParse(host[1], out int given) ? given : Settings.StationLinkPort);
|
host.Length > 1 && int.TryParse(host[1], out int given) ? given : Settings.StationLinkPort);
|
||||||
}
|
}
|
||||||
|
|
||||||
/// What another station did to its log, applied to ours. Nothing goes back
|
/// Updates that have arrived from the network and not yet been applied.
|
||||||
/// out to the network, and the contact is scored here from the rules
|
/// They are applied together on the window thread, so a resync that sends
|
||||||
/// instead of trusting what the sender put in the message.
|
/// a few hundred contacts rescores the log and redraws the windows once
|
||||||
private void TakeFromNetwork(ContactUpdate update)
|
/// per batch rather than once per contact.
|
||||||
|
private readonly ConcurrentQueue<ContactUpdate> fromNetwork = new();
|
||||||
|
|
||||||
|
private int networkDrainPosted;
|
||||||
|
|
||||||
|
private void QueueFromNetwork(ContactUpdate update)
|
||||||
|
{
|
||||||
|
fromNetwork.Enqueue(update);
|
||||||
|
if (Interlocked.Exchange(ref networkDrainPosted, 1) == 0)
|
||||||
|
{
|
||||||
|
Dispatcher.UIThread.Post(TakeQueuedFromNetwork);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
private void TakeQueuedFromNetwork()
|
||||||
|
{
|
||||||
|
// cleared first: an update that arrives while this runs posts a new drain
|
||||||
|
Interlocked.Exchange(ref networkDrainPosted, 0);
|
||||||
|
List<ContactUpdate> batch = [];
|
||||||
|
while (fromNetwork.TryDequeue(out ContactUpdate? update))
|
||||||
|
{
|
||||||
|
batch.Add(update);
|
||||||
|
}
|
||||||
|
TakeFromNetwork(batch);
|
||||||
|
}
|
||||||
|
|
||||||
|
/// What other stations did to their logs, applied to ours in the order it
|
||||||
|
/// arrived. Nothing goes back out to the network, and the contacts are
|
||||||
|
/// scored here from the rules instead of trusting what the sender put in
|
||||||
|
/// the message.
|
||||||
|
private void TakeFromNetwork(IReadOnlyList<ContactUpdate> updates)
|
||||||
{
|
{
|
||||||
if (Logging is null)
|
if (Logging is null)
|
||||||
{
|
{
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
if (!Settings.LogQsosAtAllStations && update is not ContactsCleared)
|
List<Qso> logged = [];
|
||||||
|
foreach (ContactUpdate update in updates)
|
||||||
{
|
{
|
||||||
return;
|
if (!Settings.LogQsosAtAllStations && update is not ContactsCleared)
|
||||||
}
|
{
|
||||||
switch (update)
|
continue;
|
||||||
{
|
}
|
||||||
case ContactLogged logged:
|
if (update is ContactLogged contact)
|
||||||
Logging.AddFromNetwork(logged.Qso);
|
{
|
||||||
break;
|
logged.Add(contact.Qso);
|
||||||
case ContactReplaced replaced:
|
continue;
|
||||||
Logging.ReplaceFromNetwork(replaced.Qso, replaced.OldCall, replaced.OldTimestampUtc);
|
}
|
||||||
break;
|
// a run of new contacts is applied before the update that follows
|
||||||
case ContactDeleted deleted:
|
// it, because that update may be a clear or an edit of one of them
|
||||||
Logging.DeleteFromNetwork(deleted.Id, deleted.Call, deleted.TimestampUtc);
|
Logging.AddRangeFromNetwork(logged);
|
||||||
break;
|
logged.Clear();
|
||||||
case ContactsCleared cleared:
|
switch (update)
|
||||||
Logging.ClearFromNetwork(cleared.StationName, cleared.FromUtc, cleared.Length);
|
{
|
||||||
break;
|
case ContactReplaced replaced:
|
||||||
|
Logging.ReplaceFromNetwork(replaced.Qso, replaced.OldCall, replaced.OldTimestampUtc);
|
||||||
|
break;
|
||||||
|
case ContactDeleted deleted:
|
||||||
|
Logging.DeleteFromNetwork(deleted.Id, deleted.Call, deleted.TimestampUtc);
|
||||||
|
break;
|
||||||
|
case ContactsCleared cleared:
|
||||||
|
Logging.ClearFromNetwork(cleared.StationName, cleared.FromUtc, cleared.Length);
|
||||||
|
break;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
Logging.AddRangeFromNetwork(logged);
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Starts, restarts or stops the keyer, following what the settings say.
|
/// Starts, restarts or stops the keyer, following what the settings say.
|
||||||
|
|||||||
@@ -127,12 +127,28 @@ public sealed class ContestLog
|
|||||||
Rebuild();
|
Rebuild();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// `Insert` for several contacts, scoring the log once instead of once per
|
||||||
|
/// contact. A resync sends a few hundred in a burst.
|
||||||
|
public void InsertRange(IEnumerable<Qso> arrived)
|
||||||
|
{
|
||||||
|
qsos.AddRange(arrived);
|
||||||
|
Rebuild();
|
||||||
|
}
|
||||||
|
|
||||||
public void Remove(string id)
|
public void Remove(string id)
|
||||||
{
|
{
|
||||||
qsos.RemoveAll(q => q.Id == id);
|
qsos.RemoveAll(q => q.Id == id);
|
||||||
Rebuild();
|
Rebuild();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// `Remove` for several contacts, scoring the log once.
|
||||||
|
public void RemoveRange(IReadOnlyCollection<string> ids)
|
||||||
|
{
|
||||||
|
HashSet<string> gone = [.. ids];
|
||||||
|
qsos.RemoveAll(q => gone.Contains(q.Id));
|
||||||
|
Rebuild();
|
||||||
|
}
|
||||||
|
|
||||||
public void Replace(Qso qso)
|
public void Replace(Qso qso)
|
||||||
{
|
{
|
||||||
int at = qsos.FindIndex(q => q.Id == qso.Id);
|
int at = qsos.FindIndex(q => q.Id == qso.Id);
|
||||||
|
|||||||
@@ -54,6 +54,7 @@ public sealed class StationLink : IDisposable
|
|||||||
private readonly string computerName;
|
private readonly string computerName;
|
||||||
private readonly string version;
|
private readonly string version;
|
||||||
private readonly CancellationTokenSource stopping = new();
|
private readonly CancellationTokenSource stopping = new();
|
||||||
|
private bool disposed;
|
||||||
private readonly ConcurrentDictionary<string, NetworkedStation> stations = new(StringComparer.OrdinalIgnoreCase);
|
private readonly ConcurrentDictionary<string, NetworkedStation> stations = new(StringComparer.OrdinalIgnoreCase);
|
||||||
private readonly ConcurrentDictionary<string, TcpClient> writers = new(StringComparer.OrdinalIgnoreCase);
|
private readonly ConcurrentDictionary<string, TcpClient> writers = new(StringComparer.OrdinalIgnoreCase);
|
||||||
private readonly NetworkedStation mine;
|
private readonly NetworkedStation mine;
|
||||||
@@ -496,6 +497,11 @@ public sealed class StationLink : IDisposable
|
|||||||
|
|
||||||
public void Dispose()
|
public void Dispose()
|
||||||
{
|
{
|
||||||
|
if (disposed)
|
||||||
|
{
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
disposed = true;
|
||||||
stopping.Cancel();
|
stopping.Cancel();
|
||||||
foreach (TcpClient writer in writers.Values)
|
foreach (TcpClient writer in writers.Values)
|
||||||
{
|
{
|
||||||
|
|||||||
@@ -131,10 +131,10 @@ public sealed class ContestSession
|
|||||||
foreach (Qso qso in gone)
|
foreach (Qso qso in gone)
|
||||||
{
|
{
|
||||||
store.Delete(qso.Id);
|
store.Delete(qso.Id);
|
||||||
Log.Remove(qso.Id);
|
|
||||||
}
|
}
|
||||||
if (gone.Count > 0)
|
if (gone.Count > 0)
|
||||||
{
|
{
|
||||||
|
Log.RemoveRange([.. gone.Select(q => q.Id)]);
|
||||||
Changed?.Invoke(this, EventArgs.Empty);
|
Changed?.Invoke(this, EventArgs.Empty);
|
||||||
}
|
}
|
||||||
return gone.Count;
|
return gone.Count;
|
||||||
@@ -208,6 +208,34 @@ public sealed class ContestSession
|
|||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// `AddFromNetwork` for a burst of contacts, which is what a resync sends.
|
||||||
|
/// The log is scored and `Changed` is raised once for the lot, because
|
||||||
|
/// doing either per contact keeps the window thread busy for minutes on a
|
||||||
|
/// log of a few thousand. Returns how many were new.
|
||||||
|
public int AddRangeFromNetwork(IEnumerable<Qso> arriving)
|
||||||
|
{
|
||||||
|
HashSet<string> held = [.. Log.Qsos.Select(q => q.Id)];
|
||||||
|
List<Qso> stored = [];
|
||||||
|
foreach (Qso qso in arriving)
|
||||||
|
{
|
||||||
|
if (held.Add(qso.Id))
|
||||||
|
{
|
||||||
|
stored.Add(store.Add(FromNetwork(qso)));
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if (stored.Count == 0)
|
||||||
|
{
|
||||||
|
return 0;
|
||||||
|
}
|
||||||
|
Log.InsertRange(stored);
|
||||||
|
foreach (Qso qso in stored)
|
||||||
|
{
|
||||||
|
RecordNumber(qso);
|
||||||
|
}
|
||||||
|
Changed?.Invoke(this, EventArgs.Empty);
|
||||||
|
return stored.Count;
|
||||||
|
}
|
||||||
|
|
||||||
/// An edit made at another station. False when we do not hold the contact
|
/// An edit made at another station. False when we do not hold the contact
|
||||||
/// it changes.
|
/// it changes.
|
||||||
public bool ReplaceFromNetwork(Qso qso, string oldCall, DateTime oldTimestampUtc)
|
public bool ReplaceFromNetwork(Qso qso, string oldCall, DateTime oldTimestampUtc)
|
||||||
|
|||||||
@@ -166,6 +166,23 @@ public class NetworkedContactsTests
|
|||||||
Assert.Equal(1, session.Rescore(Start.AddMinutes(60)));
|
Assert.Equal(1, session.Rescore(Start.AddMinutes(60)));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// A resync sends its contacts in a burst, and they are scored together.
|
||||||
|
[Fact]
|
||||||
|
public void ABurstOfContactsIsLoggedOnceAndRaisesOneChange()
|
||||||
|
{
|
||||||
|
(ContestSession session, FakeLogStore store) = Station();
|
||||||
|
int changes = 0;
|
||||||
|
session.Changed += (_, _) => changes++;
|
||||||
|
|
||||||
|
int added = session.AddRangeFromNetwork(
|
||||||
|
[Arriving("JA1XYZ", 1), Arriving("JA2XYZ", 2), Arriving("JA1XYZ", 1)]);
|
||||||
|
|
||||||
|
Assert.Equal(2, added);
|
||||||
|
Assert.Equal(1, changes);
|
||||||
|
Assert.Equal(2, store.Qsos(session.Instance.ContestNumber).Count);
|
||||||
|
Assert.Equal(2, session.Log.Qsos.Count);
|
||||||
|
}
|
||||||
|
|
||||||
/// A resync drops the block it is about to be sent again, so the contacts
|
/// A resync drops the block it is about to be sent again, so the contacts
|
||||||
/// are replaced rather than added to.
|
/// are replaced rather than added to.
|
||||||
[Fact]
|
[Fact]
|
||||||
|
|||||||
Reference in New Issue
Block a user