0

I need to lock a couple of variables in my event handler so that separate threads cannot access them at the same time. After researching SO, I learned that I should lock on a static private object and wrap my code around that lock. But my question is related to how much code can be put inside the lock. Below is my event handler code, but I am inclined to wrap the entire code inside the lock statement, but something tells me there is too much code being wrapped.

Are there guidelines to this? As you can see, the code updates an important variable (currentNumberOfHighSeverityAlarms). This must be managed by one thread at a time. It is at the beginning of the code and at the end. Is it ok to wrap the entire try/catch into a lock statement? I'm just not sure where to put the lock statement exactly.

    private void AlarmList_WindowDataChanged(object sender, AlsWindowDataChangeEventArgs e)
    {
        try
        {
            if (e.WindowDataItems != null && e.TotalSize > 0)
            {
                var previousNumberOfHighSeverityAlarms = currentNumberOfHighSeverityAlarms;
                currentNumberOfHighSeverityAlarms = e.WindowDataItems.Count(
                    al => Convert.ToInt32(al.Fields[8]) >= m_highSeverityRangeMinimum
                          && Convert.ToInt32(al.Fields[8]) <= m_highSeverityRangeMaximum); 

                m_alarmManagementService.HighPriorityUnacknowledgedAlarmsExist = currentNumberOfHighSeverityAlarms > 0;

                if (currentNumberOfHighSeverityAlarms > previousNumberOfHighSeverityAlarms)
                {
                    SoundManagerComponent.AlarmsMuted = false;

                    if (previousNumberOfHighSeverityAlarms == 0 && m_alarmListStartupCompleted == false)
                    {
                        var highSeveritySoundFile = SystemConfigurationComponent.GetSubsystemSetting("gcs", "alarmmanagementservice", "HighSeveritySoundFile").SettingValue;
                        SoundManagerComponent.SetRepeatSound(highSeveritySoundFile, TimeSpan.FromSeconds(3), SoundManagerComponent.AlarmsVolume);
                        m_alarmListStartupCompleted = true;
                        LoggingComponent.WriteApplicationLog("Alarm sound turned on. New high-severity alarms generated.", "Main Toolbar AlarmViewer");
                    }
                }

                var highSeverityAlarmsExistInAlarmViewer = e.WindowDataItems.Any(
                    al => Convert.ToInt32(al.Fields[8]) >= m_highSeverityRangeMinimum
                          && Convert.ToInt32(al.Fields[8]) <= m_highSeverityRangeMaximum);

                if (!highSeverityAlarmsExistInAlarmViewer)
                {
                    SoundManagerComponent.StopRepeatedSound();
                }
            }
            else
            {
                SoundManagerComponent.StopRepeatedSound();
                m_alarmManagementService.HighPriorityUnacknowledgedAlarmsExist = false;
                currentNumberOfHighSeverityAlarms = 0;
                LoggingComponent.WriteApplicationLog("Alarm sound turned off. No high-severity alarms exist.", "Main Toolbar AlarmViewer");
            }
        }
        catch (Exception ex)
        {
            Trace.TraceError("Error while counting the number of high-severity alarms. Ex: " + ex.Message);
        }
    }
Ray
  • 4,679
  • 10
  • 46
  • 92
  • 1
    You can put as much code inside a lock as you like, though it might pay to split it up into smaller functions. – DavidG Feb 25 '21 at 17:11
  • I agree with DavidG statement above, with this in mind, it will help reduce lock contention. – Trevor Feb 25 '21 at 17:29
  • 1
    See https://ericlippert.com/2009/03/06/locks-and-exceptions-do-not-mix/ and https://stackoverflow.com/questions/639493/in-c-sharp-how-can-i-safely-exit-a-lock-with-a-try-catch-block-inside – Charlieface Feb 25 '21 at 17:31
  • Comprehensive guidelinse on locking is far beyond SO question scope... But we already have at least one on the site... indeed it is incomplete but a start (picked as [duplicate](https://stackoverflow.com/questions/2779703/guidelines-of-when-to-use-locking)). If you can narrow down your concerns with an [edit] this probably can be re-opened. – Alexei Levenkov Feb 25 '21 at 17:57

0 Answers0