Rewrite ItemDataProvider to be more robust

* Stop locking 2+ times per operation
* Don't clone the list multiple times
* Keep the lock for the duration of the operation

Emby.Server.Implementations/LiveTv/EmbyTV/EmbyTV.cs

@@ -102,7 +102,7 @@ namespace Emby.Server.Implementations.LiveTv.EmbyTV
             _streamHelper = streamHelper;
 
             _seriesTimerProvider = new SeriesTimerManager(jsonSerializer, _logger, Path.Combine(DataPath, "seriestimers.json"));
-            _timerProvider = new TimerManager(jsonSerializer, _logger, Path.Combine(DataPath, "timers.json"), _logger);
+            _timerProvider = new TimerManager(jsonSerializer, _logger, Path.Combine(DataPath, "timers.json"));
             _timerProvider.TimerFired += _timerProvider_TimerFired;
 
             _config.NamedConfigurationUpdated += _config_NamedConfigurationUpdated;

Emby.Server.Implementations/LiveTv/EmbyTV/ItemDataProvider.cs

@@ -10,67 +10,64 @@ namespace Emby.Server.Implementations.LiveTv.EmbyTV
     public class ItemDataProvider<T>
         where T : class
     {
-        private readonly object _fileDataLock = new object();
-        private List<T> _items;
         private readonly IJsonSerializer _jsonSerializer;
-        protected readonly ILogger Logger;
         private readonly string _dataPath;
-        protected readonly Func<T, T, bool> EqualityComparer;
+        private readonly object _fileDataLock = new object();
+        private T[] _items;
 
-        public ItemDataProvider(IJsonSerializer jsonSerializer, ILogger logger, string dataPath, Func<T, T, bool> equalityComparer)
+        public ItemDataProvider(
+            IJsonSerializer jsonSerializer,
+            ILogger logger,
+            string dataPath,
+            Func<T, T, bool> equalityComparer)
         {
+            _jsonSerializer = jsonSerializer;
             Logger = logger;
             _dataPath = dataPath;
             EqualityComparer = equalityComparer;
-            _jsonSerializer = jsonSerializer;
+        }
+
+        protected ILogger Logger { get; }
+
+        protected Func<T, T, bool> EqualityComparer { get; }
+
+        private void EnsureLoaded()
+        {
+            if (_items != null)
+            {
+                return;
+            }
+
+            if (File.Exists(_dataPath))
+            {
+                Logger.LogInformation("Loading live tv data from {Path}", _dataPath);
+
+                try
+                {
+                    _items = _jsonSerializer.DeserializeFromFile<T[]>(_dataPath);
+                    return;
+                }
+                catch (Exception ex)
+                {
+                    Logger.LogError(ex, "Error deserializing {Path}", _dataPath);
+                }
+            }
+
+            _items = Array.Empty<T>();
+        }
+
+        private void SaveList()
+        {
+            Directory.CreateDirectory(Path.GetDirectoryName(_dataPath));
+            _jsonSerializer.SerializeToFile(_items, _dataPath);
         }
 
         public IReadOnlyList<T> GetAll()
         {
             lock (_fileDataLock)
             {
-                if (_items == null)
+                EnsureLoaded();
-                {
+                return (T[])_items.Clone();
-                    if (!File.Exists(_dataPath))
-                    {
-                        return new List<T>();
-                    }
-
-                    Logger.LogInformation("Loading live tv data from {0}", _dataPath);
-                    _items = GetItemsFromFile(_dataPath);
-                }
-
-                return _items.ToList();
-            }
-        }
-
-        private List<T> GetItemsFromFile(string path)
-        {
-            try
-            {
-                return _jsonSerializer.DeserializeFromFile<List<T>>(path);
-            }
-            catch (Exception ex)
-            {
-                Logger.LogError(ex, "Error deserializing {Path}", path);
-            }
-
-            return new List<T>();
-        }
-
-        private void UpdateList(List<T> newList)
-        {
-            if (newList == null)
-            {
-                throw new ArgumentNullException(nameof(newList));
-            }
-
-            Directory.CreateDirectory(Path.GetDirectoryName(_dataPath));
-
-            lock (_fileDataLock)
-            {
-                _jsonSerializer.SerializeToFile(newList, _dataPath);
-                _items = newList;
             }
         }
 
@@ -81,18 +78,20 @@ namespace Emby.Server.Implementations.LiveTv.EmbyTV
                 throw new ArgumentNullException(nameof(item));
             }
 
-            var list = GetAll().ToList();
+            lock (_fileDataLock)
-
+            {
-            var index = list.FindIndex(i => EqualityComparer(i, item));
+                EnsureLoaded();
 
+                var index = Array.FindIndex(_items, i => EqualityComparer(i, item));
                 if (index == -1)
                 {
                     throw new ArgumentException("item not found");
                 }
 
-            list[index] = item;
+                _items[index] = item;
 
-            UpdateList(list);
+                SaveList();
+            }
         }
 
         public virtual void Add(T item)
@@ -102,37 +101,58 @@ namespace Emby.Server.Implementations.LiveTv.EmbyTV
                 throw new ArgumentNullException(nameof(item));
             }
 
-            var list = GetAll().ToList();
+            lock (_fileDataLock)
-
-            if (list.Any(i => EqualityComparer(i, item)))
             {
-                throw new ArgumentException("item already exists");
+                EnsureLoaded();
+
+                if (_items.Any(i => EqualityComparer(i, item)))
+                {
+                    throw new ArgumentException("item already exists", nameof(item));
                 }
 
-            list.Add(item);
+                int oldLen = _items.Length;
+                var newList = new T[oldLen + 1];
+                _items.CopyTo(newList, 0);
+                newList[oldLen] = item;
+                _items = newList;
 
-            UpdateList(list);
+                SaveList();
+            }
         }
 
-        public void AddOrUpdate(T item)
+        public virtual void AddOrUpdate(T item)
         {
-            var list = GetAll().ToList();
+            lock (_fileDataLock)
+            {
+                EnsureLoaded();
 
-            if (!list.Any(i => EqualityComparer(i, item)))
+                int index = Array.FindIndex(_items, i => EqualityComparer(i, item));
+                if (index == -1)
                 {
-                Add(item);
+                    int oldLen = _items.Length;
+                    var newList = new T[oldLen + 1];
+                    _items.CopyTo(newList, 0);
+                    newList[oldLen] = item;
+                    _items = newList;
                 }
                 else
                 {
-                Update(item);
+                    _items[index] = item;
+                }
+
+                SaveList();
             }
         }
 
         public virtual void Delete(T item)
         {
-            var list = GetAll().Where(i => !EqualityComparer(i, item)).ToList();
+            lock (_fileDataLock)
+            {
+                EnsureLoaded();
+                _items = _items.Where(i => !EqualityComparer(i, item)).ToArray();
 
-            UpdateList(list);
+                SaveList();
+            }
         }
     }
 }

Emby.Server.Implementations/LiveTv/EmbyTV/TimerManager.cs

@@ -14,21 +14,19 @@ namespace Emby.Server.Implementations.LiveTv.EmbyTV
     public class TimerManager : ItemDataProvider<TimerInfo>
     {
         private readonly ConcurrentDictionary<string, Timer> _timers = new ConcurrentDictionary<string, Timer>(StringComparer.OrdinalIgnoreCase);
-        private readonly ILogger _logger;
 
-        public event EventHandler<GenericEventArgs<TimerInfo>> TimerFired;
+        public TimerManager(IJsonSerializer jsonSerializer, ILogger logger, string dataPath)
-
-        public TimerManager(IJsonSerializer jsonSerializer, ILogger logger, string dataPath, ILogger logger1)
             : base(jsonSerializer, logger, dataPath, (r1, r2) => string.Equals(r1.Id, r2.Id, StringComparison.OrdinalIgnoreCase))
         {
-            _logger = logger1;
         }
 
+        public event EventHandler<GenericEventArgs<TimerInfo>> TimerFired;
+
         public void RestartTimers()
         {
             StopTimers();
 
-            foreach (var item in GetAll().ToList())
+            foreach (var item in GetAll())
             {
                 AddOrUpdateSystemTimer(item);
             }
@@ -64,16 +62,13 @@ namespace Emby.Server.Implementations.LiveTv.EmbyTV
                 return;
             }
 
-            var list = GetAll().ToList();
+            base.AddOrUpdate(item);
+        }
 
-            if (!list.Any(i => EqualityComparer(i, item)))
+        public override void AddOrUpdate(TimerInfo item)
         {
-                base.Add(item);
+            base.AddOrUpdate(item);
-            }
+            AddOrUpdateSystemTimer(item);
-            else
-            {
-                base.Update(item);
-            }
         }
 
         public override void Add(TimerInfo item)
@@ -89,8 +84,8 @@ namespace Emby.Server.Implementations.LiveTv.EmbyTV
 
         private static bool ShouldStartTimer(TimerInfo item)
         {
-            if (item.Status == RecordingStatus.Completed ||
+            if (item.Status == RecordingStatus.Completed
-                item.Status == RecordingStatus.Cancelled)
+                || item.Status == RecordingStatus.Cancelled)
             {
                 return false;
             }
@@ -126,12 +121,16 @@ namespace Emby.Server.Implementations.LiveTv.EmbyTV
 
             if (_timers.TryAdd(item.Id, timer))
             {
-                _logger.LogInformation("Creating recording timer for {id}, {name}. Timer will fire in {minutes} minutes", item.Id, item.Name, dueTime.TotalMinutes.ToString(CultureInfo.InvariantCulture));
+                Logger.LogInformation(
+                    "Creating recording timer for {Id}, {Name}. Timer will fire in {Minutes} minutes",
+                    item.Id,
+                    item.Name,
+                    dueTime.TotalMinutes.ToString(CultureInfo.InvariantCulture));
             }
             else
             {
                 timer.Dispose();
-                _logger.LogWarning("Timer already exists for item {id}", item.Id);
+                Logger.LogWarning("Timer already exists for item {Id}", item.Id);
             }
         }