fix: move logger calls outside locks, protect SignalsSeen reads

Self-review fixes for the EnrollmentTracker locking commit:
- NotifyDesktopArrived: move _logger.Debug outside lock (no I/O under lock)
- CollectDeviceInfoAtFinalizingSetup: same pattern
- EmitEnrollmentFailed: wrap _stateData.SignalsSeen copy in lock via
  SnapshotSignalsSeen() to prevent InvalidOperationException if
  RecordSignal modifies the list concurrently
- WriteFinalStatus: same fix for SignalsSeen access

https://claude.ai/code/session_01EWzmYXFmXUaPpeksi6WGKZ
This commit is contained in:
Claude
2026-03-30 11:11:57 +00:00
parent 0130609a2d
commit de411ba4ac
2 changed files with 36 additions and 15 deletions
@@ -345,7 +345,7 @@ namespace AutopilotMonitor.Agent.Core.Monitoring.Tracking
{
{ "failureType", failureType },
{ "failureSource", failureSource },
{ "signalsSeen", new List<string>(_stateData.SignalsSeen) },
{ "signalsSeen", SnapshotSignalsSeen() },
{ "agentUptimeSeconds", (DateTime.UtcNow - _agentStartTimeUtc).TotalSeconds }
},
ImmediateUpload = true
@@ -601,6 +601,17 @@ namespace AutopilotMonitor.Agent.Core.Monitoring.Tracking
_logger.Debug($"EnrollmentTracker: signal '{signal}' already recorded — deduplicated");
}
/// <summary>
/// Returns a thread-safe snapshot of _stateData.SignalsSeen for use in event data.
/// </summary>
private List<string> SnapshotSignalsSeen()
{
lock (_stateLock)
{
return new List<string>(_stateData.SignalsSeen);
}
}
// ===== Unified Completion Logic =====
/// <summary>
@@ -820,18 +831,24 @@ namespace AutopilotMonitor.Agent.Core.Monitoring.Tracking
/// </summary>
public void NotifyDesktopArrived()
{
bool alreadyArrived;
lock (_stateLock)
{
if (_desktopArrived)
alreadyArrived = _desktopArrived;
if (!alreadyArrived)
{
_logger.Debug("EnrollmentTracker: NotifyDesktopArrived called but already arrived — ignoring");
return;
_desktopArrived = true;
_stateData.DesktopArrived = true;
_stateData.DesktopArrivedUtc = DateTime.UtcNow;
}
_desktopArrived = true;
_stateData.DesktopArrived = true;
_stateData.DesktopArrivedUtc = DateTime.UtcNow;
}
if (alreadyArrived)
{
_logger.Debug("EnrollmentTracker: NotifyDesktopArrived called but already arrived — ignoring");
return;
}
RecordSignal("desktop_arrived");
_logger.Info("EnrollmentTracker: Desktop arrival notified");
@@ -47,14 +47,18 @@ namespace AutopilotMonitor.Agent.Core.Monitoring.Tracking
private void CollectDeviceInfoAtFinalizingSetup(string triggerReason)
{
bool alreadyCollected;
lock (_stateLock)
{
if (_finalDeviceInfoCollected)
{
_logger.Debug($"EnrollmentTracker: final device info already collected, skipping (trigger: {triggerReason})");
return;
}
_finalDeviceInfoCollected = true;
alreadyCollected = _finalDeviceInfoCollected;
if (!alreadyCollected)
_finalDeviceInfoCollected = true;
}
if (alreadyCollected)
{
_logger.Debug($"EnrollmentTracker: final device info already collected, skipping (trigger: {triggerReason})");
return;
}
_logger.Info($"EnrollmentTracker: triggering final device info collection at FinalizingSetup (trigger: {triggerReason})");
CollectDeviceInfoAtEnd();
@@ -338,7 +342,7 @@ namespace AutopilotMonitor.Agent.Core.Monitoring.Tracking
{ "helloOutcome", _espAndHelloTracker?.HelloOutcome ?? "unknown" },
{ "enrollmentType", _enrollmentType ?? "unknown" },
{ "agentUptimeSeconds", (DateTime.UtcNow - _agentStartTimeUtc).TotalSeconds },
{ "signalsSeen", _stateData.SignalsSeen ?? new List<string>() },
{ "signalsSeen", SnapshotSignalsSeen() },
{ "signalTimestamps", signalTimestamps },
{ "appSummary", new Dictionary<string, object>
{