Core - Corrected disposable pattern usage

Core - Further nullable refactoring..

src/Artemis.Core/Plugins/LayerBrushes/Internal/BaseLayerBrush.cs

@@ -97,19 +97,32 @@ namespace Artemis.Core.LayerBrushes
 
         internal abstract void InternalRender(SKCanvas canvas, SKRect path, SKPaint paint);
 
-        internal virtual void Dispose(bool disposing)
+        #region IDisposable
+
+        /// <summary>
+        ///     Releases the unmanaged resources used by the object and optionally releases the managed resources.
+        /// </summary>
+        /// <param name="disposing">
+        ///     <see langword="true" /> to release both managed and unmanaged resources;
+        ///     <see langword="false" /> to release only unmanaged resources.
+        /// </param>
+        protected virtual void Dispose(bool disposing)
         {
+            if (disposing)
+            {
+                DisableLayerBrush();
+                BaseProperties?.Dispose();
+            }
         }
 
         /// <inheritdoc />
         public void Dispose()
         {
-            DisableLayerBrush();
-            BaseProperties?.Dispose();
-
             Dispose(true);
             GC.SuppressFinalize(this);
         }
+
+        #endregion
     }
 
     /// <summary>

src/Artemis.Core/Plugins/LayerBrushes/Internal/PropertiesLayerBrush.cs

@@ -26,7 +26,7 @@ namespace Artemis.Core.LayerBrushes
             {
                 // I imagine a null reference here can be confusing, so lets throw an exception explaining what to do
                 if (_properties == null)
-                    throw new ArtemisPluginException("Cannot access brush properties until OnPropertiesInitialized has been called");
+                    throw new InvalidOperationException("Cannot access brush properties until OnPropertiesInitialized has been called");
                 return _properties;
             }
             internal set => _properties = value;

src/Artemis.Core/Plugins/LayerBrushes/LayerBrushProvider.cs

@@ -46,7 +46,7 @@ namespace Artemis.Core.LayerBrushes
             LayerBrushStore.Add(descriptor);
         }
 
-        private void OnDisabled(object sender, EventArgs e)
+        private void OnDisabled(object? sender, EventArgs e)
         {
             // The store will clean up the registrations by itself, the plugin just needs to clear its own list
             _layerBrushDescriptors.Clear();

src/Artemis.Core/Plugins/LayerBrushes/RgbNetLayerBrush.cs

@@ -56,7 +56,10 @@ namespace Artemis.Core.LayerBrushes
             UpdateLedGroup();
         }
 
-        internal override void Dispose(bool disposing)
+        #region IDisposable
+
+        /// <inheritdoc />
+        protected override void Dispose(bool disposing)
         {
             if (disposing)
             {
@@ -67,13 +70,15 @@ namespace Artemis.Core.LayerBrushes
             base.Dispose(disposing);
         }
 
+        #endregion
+
         // Not used in this effect type
         internal override void InternalRender(SKCanvas canvas, SKRect bounds, SKPaint paint)
         {
             throw new NotImplementedException("RGB.NET layer effectes do not implement InternalRender");
         }
 
-        private void LayerOnRenderPropertiesUpdated(object sender, EventArgs e)
+        private void LayerOnRenderPropertiesUpdated(object? sender, EventArgs e)
         {
             UpdateLedGroup();
         }

src/Artemis.Core/Plugins/LayerEffects/Internal/BaseLayerEffect.cs

@@ -103,13 +103,33 @@ namespace Artemis.Core.LayerEffects
 
         internal string PropertyRootPath => $"LayerEffect.{EntityId}.{GetType().Name}.";
 
+        #region IDisposable
+
+        /// <summary>
+        ///     Releases the unmanaged resources used by the object and optionally releases the managed resources.
+        /// </summary>
+        /// <param name="disposing">
+        ///     <see langword="true" /> to release both managed and unmanaged resources;
+        ///     <see langword="false" /> to release only unmanaged resources.
+        /// </param>
+        protected virtual void Dispose(bool disposing)
+        {
+            if (disposing)
+            {
+                DisableLayerEffect();
+                BaseProperties?.Dispose();
+            }
+        }
+
         /// <inheritdoc />
         public void Dispose()
         {
-            DisableLayerEffect();
-            BaseProperties?.Dispose();
+            Dispose(true);
+            GC.SuppressFinalize(this);
         }
 
+        #endregion
+
         /// <summary>
         ///     Called when the layer effect is activated
         /// </summary>

src/Artemis.Core/Plugins/LayerEffects/LayerEffect.cs

@@ -27,7 +27,7 @@ namespace Artemis.Core.LayerEffects
             {
                 // I imagine a null reference here can be confusing, so lets throw an exception explaining what to do
                 if (_properties == null)
-                    throw new ArtemisPluginException("Cannot access effect properties until OnPropertiesInitialized has been called");
+                    throw new InvalidOperationException("Cannot access effect properties until OnPropertiesInitialized has been called");
                 return _properties;
             }
             internal set => _properties = value;

src/Artemis.Core/Plugins/LayerEffects/LayerEffectProvider.cs

@@ -46,7 +46,7 @@ namespace Artemis.Core.LayerEffects
             LayerEffectStore.Add(descriptor);
         }
 
-        private void OnDisabled(object sender, EventArgs e)
+        private void OnDisabled(object? sender, EventArgs e)
         {
             // The store will clean up the registrations by itself, the plugin just needs to clear its own list
             _layerEffectDescriptors.Clear();

src/Artemis.Core/Plugins/Modules/Module.cs

@@ -140,7 +140,7 @@ namespace Artemis.Core.Modules
         internal DataModel? InternalDataModel { get; set; }
 
         internal bool InternalExpandsMainDataModel { get; set; }
-        internal ModuleSettingsEntity? Entity { get; set; }
+        internal ModuleSettingsEntity? SettingsEntity { get; set; }
 
         /// <summary>
         ///     Called each frame when the module should update
@@ -233,12 +233,12 @@ namespace Artemis.Core.Modules
 
         internal void ApplyToEntity()
         {
-            if (Entity == null)
-                Entity = new ModuleSettingsEntity();
+            if (SettingsEntity == null)
+                SettingsEntity = new ModuleSettingsEntity();
 
-            Entity.ModuleId = Id;
-            Entity.PriorityCategory = (int) PriorityCategory;
-            Entity.Priority = Priority;
+            SettingsEntity.ModuleId = Id;
+            SettingsEntity.PriorityCategory = (int) PriorityCategory;
+            SettingsEntity.Priority = Priority;
         }
     }
 

src/Artemis.Core/Plugins/Modules/ProfileModule.cs

@@ -96,6 +96,7 @@ namespace Artemis.Core.Modules
         ///     Gets a list of all properties ignored at runtime using <c>IgnoreProperty(x => x.y)</c>
         /// </summary>
         protected internal readonly List<PropertyInfo> HiddenPropertiesList = new List<PropertyInfo>();
+        private readonly object _lock = new object();
 
         /// <summary>
         ///     Creates a new instance of the <see cref="ProfileModule" /> class
@@ -154,7 +155,7 @@ namespace Artemis.Core.Modules
             if (IsUpdateAllowed)
                 Update(deltaTime);
 
-            lock (this)
+            lock (_lock)
             {
                 OpacityOverride = AnimatingProfileChange
                     ? Math.Max(0, OpacityOverride - 0.1)
@@ -172,7 +173,7 @@ namespace Artemis.Core.Modules
         {
             Render(deltaTime, surface, canvas, canvasInfo);
 
-            lock (this)
+            lock (_lock)
             {
                 // Render the profile
                 ActiveProfile?.Render(canvas);
@@ -210,7 +211,7 @@ namespace Artemis.Core.Modules
             if (!IsActivated)
                 throw new ArtemisCoreException("Cannot activate a profile on a deactivated module");
 
-            lock (this)
+            lock (_lock)
             {
                 if (profile == ActiveProfile)
                     return;

src/Artemis.Core/Plugins/Plugin.cs

@@ -160,19 +160,39 @@ namespace Artemis.Core
             }
         }
 
+        #region IDisposable
+
+        /// <summary>
+        ///     Releases the unmanaged resources used by the object and optionally releases the managed resources.
+        /// </summary>
+        /// <param name="disposing">
+        ///     <see langword="true" /> to release both managed and unmanaged resources;
+        ///     <see langword="false" /> to release only unmanaged resources.
+        /// </param>
+        protected virtual void Dispose(bool disposing)
+        {
+            if (disposing)
+            {
+                foreach (PluginFeature feature in Features)
+                    feature.Dispose();
+
+                Kernel?.Dispose();
+                PluginLoader?.Dispose();
+
+                _features.Clear();
+                SetEnabled(false);
+            }
+        }
+        
         /// <inheritdoc />
         public void Dispose()
         {
-            foreach (PluginFeature feature in Features)
-                feature.Dispose();
-
-            Kernel?.Dispose();
-            PluginLoader?.Dispose();
-
-            _features.Clear();
-            SetEnabled(false);
+            Dispose(true);
+            GC.SuppressFinalize(this);
         }
 
+        #endregion
+        
         #region Events
 
         /// <summary>

src/Artemis.Core/Plugins/PluginFeature.cs

@@ -127,10 +127,30 @@ namespace Artemis.Core
             Disable();
         }
 
+        #region IDisposable
+
+        /// <summary>
+        ///     Releases the unmanaged resources used by the plugin feature and optionally releases the managed resources.
+        /// </summary>
+        /// <param name="disposing">
+        ///     <see langword="true" /> to release both managed and unmanaged resources;
+        ///     <see langword="false" /> to release only unmanaged resources.
+        /// </param>
+        protected virtual void Dispose(bool disposing)
+        {
+            if (disposing)
+            {
+                Disable();
+            }
+        }
+
+        #endregion
+
         /// <inheritdoc />
         public void Dispose()
         {
-            Disable();
+            Dispose(true);
+            GC.SuppressFinalize(this);
         }
 
         #region Loading

src/Artemis.Core/Plugins/TimedUpdateRegistration.cs

@@ -13,6 +13,7 @@ namespace Artemis.Core
         private DateTime _lastEvent;
         private Timer _timer;
         private bool _disposed;
+        private readonly object _lock = new object();
 
         internal TimedUpdateRegistration(PluginFeature feature, TimeSpan interval, Action<double> action)
         {
@@ -68,7 +69,7 @@ namespace Artemis.Core
             if (_disposed)
                 throw new ObjectDisposedException("TimedUpdateRegistration");
 
-            lock (this)
+            lock (_lock)
             {
                 if (!Feature.IsEnabled)
                     throw new ArtemisPluginException("Cannot start a timed update for a disabled plugin feature");
@@ -92,7 +93,7 @@ namespace Artemis.Core
             if (_disposed)
                 throw new ObjectDisposedException("TimedUpdateRegistration");
 
-            lock (this)
+            lock (_lock)
             {
                 if (_timer == null)
                     return;
@@ -109,7 +110,7 @@ namespace Artemis.Core
             if (!Feature.IsEnabled)
                 return;
 
-            lock (this)
+            lock (_lock)
             {
                 TimeSpan interval = DateTime.Now - _lastEvent;
                 _lastEvent = DateTime.Now;
@@ -138,15 +139,35 @@ namespace Artemis.Core
             Stop();
         }
 
+        #region IDisposable
+
+        /// <summary>
+        ///     Releases the unmanaged resources used by the object and optionally releases the managed resources.
+        /// </summary>
+        /// <param name="disposing">
+        ///     <see langword="true" /> to release both managed and unmanaged resources;
+        ///     <see langword="false" /> to release only unmanaged resources.
+        /// </param>
+        protected virtual void Dispose(bool disposing)
+        {
+            if (disposing)
+            {
+                Stop();
+
+                Feature.Enabled -= FeatureOnEnabled;
+                Feature.Disabled -= FeatureOnDisabled;
+
+                _disposed = true;
+            }
+        }
+
         /// <inheritdoc />
         public void Dispose()
         {
-            Stop();
-
-            Feature.Enabled -= FeatureOnEnabled;
-            Feature.Disabled -= FeatureOnDisabled;
-
-            _disposed = true;
+            Dispose(true);
+            GC.SuppressFinalize(this);
         }
+
+        #endregion
     }
 }