C#事件处理中私有变量的多线程使用合理性问询

API 探路者

2026-7-1

问题：多线程环境下告警计数的lock实现是否合理？

我通过如下事件处理程序订阅告警更新事件，程序会接收包含严重等级的告警列表，当新列表中高严重等级可听告警的数量多于旧列表时触发声音提示。我把之前的高严重等级可听告警数存入私有变量m_previousNumberOfHighSeverityAudibleAlarms用于后续比较，考虑到事件处理程序可能由工作线程执行，担心多线程下这个变量的正确性，所以用lock包裹了核心处理逻辑，请问这个实现是否合理？

private int m_previousNumberOfHighSeverityAudibleAlarms = 0;

private void AlarmList_WindowDataChanged(object sender, AlsWindowDataChangeEventArgs e)
{
    lock (syncObj)
    {
        try
        {
            if (e.WindowDataItems != null && e.TotalSize > 0)
            {
                var previousNumberOfHighSeverityAudibleAlarms = m_previousNumberOfHighSeverityAudibleAlarms;
                
                currentNumberOfHighSeverityAudibleAlarms = e.WindowDataItems.Count(
                        al => Convert.ToInt32(al.Fields[8]) >= m_highSeverityAudibleRangeMinimum    // High Severity Audible Range Minimum here.
                              && Convert.ToInt32(al.Fields[8]) <= m_highSeverityRangeMaximum);      

               
                    if (currentNumberOfHighSeverityAudibleAlarms > previousNumberOfHighSeverityAudibleAlarms)
                    {
                        SoundManagerComponent.AlarmsMuted = false;
                        var highSeveritySoundFile = SystemConfigurationComponent.GetSubsystemSetting("gcs", "alarmmanagementservice", "HighSeveritySoundFile").SettingValue;
                        SoundManagerComponent.SetRepeatSound(highSeveritySoundFile, TimeSpan.FromSeconds(3), SoundManagerComponent.AlarmsVolume);
                    }
                    
                    m_previousNumberOfHighSeverityAlarms = currentNumberOfHighSeverityAudibleAlarms;
                    m_previousNumberOfHighSeverityAudibleAlarms = currentNumberOfHighSeverityAudibleAlarms;
                }
                else
                {
                    SoundManagerComponent.StopRepeatedSound();
                    // This property is updated here because the AlarmManagementService's WindowDatChanged event is unreliable (AGUI product CR atvcm01122434)
                    m_alarmManagementService.HighPriorityUnacknowledgedAlarmsExist = false;
                    currentNumberOfHighSeverityAlarms = 0;
                    currentNumberOfHighSeverityAudibleAlarms = 0;
                                       
                }
            }
            catch (Exception ex)
            {
                Trace.TraceError("Error while counting the number of high-severity alarms. Ex: " + ex.Message);
            }
    }
}

分析与回答

整体思路是合理的：用lock保护共享变量m_previousNumberOfHighSeverityAudibleAlarms的读写操作，避免多线程下的竞态条件，这部分核心逻辑是正确的。但代码里还有几个需要修正和优化的点：

syncObj的定义必须规范：代码中用到的syncObj必须是private static readonly object syncObj = new object();。如果是实例变量或非readonly对象，可能导致锁失效——比如多个实例各自持有独立的锁，或者对象被替换后锁失去作用。
未声明变量的问题：代码里的currentNumberOfHighSeverityAudibleAlarms和currentNumberOfHighSeverityAlarms没有声明。如果是成员变量，会引入新的线程安全问题（这些变量的读写未被锁保护）；建议直接在lock块内声明为局部变量，避免共享。
笔误与冗余代码：m_previousNumberOfHighSeverityAlarms未在开头定义，属于笔误，且和m_previousNumberOfHighSeverityAudibleAlarms重复赋值，删掉其中一个即可。
lock块范围过大：当前lock块包含了声音播放、配置读取等耗时操作，会拉长锁的持有时间，降低并发性能。建议只在lock内完成计数比较、共享变量更新的核心逻辑，把播放/停止声音的操作移到lock外，示例：
```
bool needPlayAlarm = false;
lock(syncObj)
{
    // 计算当前计数、比较新旧值、更新previous变量
    needPlayAlarm = currentCount > previousCount;
}
if(needPlayAlarm)
{
    // 播放声音的逻辑
}
```

性能优化：Convert.ToInt32(al.Fields[8])在Count的lambda中被调用两次，可缓存这个值减少重复计算：

al => {
    int severity = Convert.ToInt32(al.Fields[8]);
    return severity >= m_highSeverityAudibleRangeMinimum && severity <= m_highSeverityRangeMaximum;
}

异常处理细化：Convert.ToInt32可能抛出格式异常，建议单独捕获这类特定异常，避免笼统处理所有异常，方便排查问题。
else分支逻辑确认：当e.WindowDataItems为空或TotalSize为0时直接把HighPriorityUnacknowledgedAlarmsExist设为false，虽然你注释了事件不可靠，但要确认是否符合业务需求——比如是否存在告警列表为空但仍有未确认高优先级告警的场景？

内容的提问来源于stack exchange，提问作者Ray