Merge pull request #1472 from peppy/memory-fixes

Fix memory leaks post game exit

osu-framework.sln.DotSettings

@@ -598,6 +598,7 @@ Licensed under the MIT Licence - https://raw.githubusercontent.com/ppy/osu-frame
 	<s:String x:Key="/Default/CodeStyle/Naming/CSharpNaming/PredefinedNamingRules/=Constants/@EntryIndexedValue">&lt;Policy Inspect="True" Prefix="" Suffix="" Style="AA_BB" /&gt;</s:String>
 	<s:String x:Key="/Default/CodeStyle/Naming/CSharpNaming/PredefinedNamingRules/=EnumMember/@EntryIndexedValue">&lt;Policy Inspect="False" Prefix="" Suffix="" Style="AaBb" /&gt;</s:String>
 	<s:String x:Key="/Default/CodeStyle/Naming/CSharpNaming/PredefinedNamingRules/=LocalConstants/@EntryIndexedValue">&lt;Policy Inspect="True" Prefix="" Suffix="" Style="aa_bb" /&gt;</s:String>
+	<s:String x:Key="/Default/CodeStyle/Naming/CSharpNaming/PredefinedNamingRules/=LocalFunctions/@EntryIndexedValue">&lt;Policy Inspect="True" Prefix="" Suffix="" Style="aaBb" /&gt;</s:String>
 	<s:String x:Key="/Default/CodeStyle/Naming/CSharpNaming/PredefinedNamingRules/=PrivateConstants/@EntryIndexedValue">&lt;Policy Inspect="True" Prefix="" Suffix="" Style="aa_bb" /&gt;</s:String>
 	<s:String x:Key="/Default/CodeStyle/Naming/CSharpNaming/PredefinedNamingRules/=PrivateInstanceFields/@EntryIndexedValue">&lt;Policy Inspect="True" Prefix="" Suffix="" Style="aaBb"&gt;&lt;ExtraRule Prefix="_" Suffix="" Style="aaBb" /&gt;&lt;/Policy&gt;</s:String>
 	<s:String x:Key="/Default/CodeStyle/Naming/CSharpNaming/PredefinedNamingRules/=PrivateStaticFields/@EntryIndexedValue">&lt;Policy Inspect="True" Prefix="" Suffix="" Style="aaBb" /&gt;</s:String>

osu.Framework.Tests/IO/TestSortedListSerialization.cs

@@ -48,12 +48,12 @@ public void TestEmptySerialization()
         [Test]
         public void TestCustomComparer()
         {
-            int Compare(int i1, int i2) => i2.CompareTo(i1);
+            int compare(int i1, int i2) => i2.CompareTo(i1);
 
-            var original = new SortedList<int>(Compare);
+            var original = new SortedList<int>(compare);
             original.AddRange(new[] { 1, 2, 3, 4, 5, 6 });
 
-            var deserialized = new SortedList<int>(Compare);
+            var deserialized = new SortedList<int>(compare);
             JsonConvert.PopulateObject(JsonConvert.SerializeObject(original), deserialized);
 
             Assert.AreEqual(original.Count, deserialized.Count, "Counts are not equal");

osu.Framework/Graphics/Containers/CompositeDrawable.cs

@@ -98,7 +98,6 @@ protected override void Dispose(bool isDisposing)
             InternalChildren?.ForEach(c => c.Dispose());
 
             OnAutoSize = null;
-            schedulerAfterChildren?.Dispose();
             schedulerAfterChildren = null;
 
             base.Dispose(isDisposing);

osu.Framework/Graphics/Drawable.cs

@@ -87,7 +87,6 @@ private void dispose(bool isDisposing)
 
             Parent = null;
 
-            scheduler?.Dispose();
             scheduler = null;
 
             OnUpdate = null;

osu.Framework/Graphics/OpenGL/GLWrapper.cs

@@ -46,13 +46,13 @@ internal static class GLWrapper
 
         public static bool IsInitialized { get; private set; }
 
-        private static GameHost host;
+        private static WeakReference<GameHost> host;
 
         internal static void Initialize(GameHost host)
         {
             if (IsInitialized) return;
 
-            GLWrapper.host = host;
+            GLWrapper.host = new WeakReference<GameHost>(host);
             reset_scheduler.SetCurrentThread();
 
             MaxTextureSize = Math.Min(4096, GL.GetInteger(GetPName.MaxTextureSize));
@@ -67,7 +67,8 @@ internal static void Initialize(GameHost host)
 
         internal static void ScheduleDisposal(Action disposalAction)
         {
-            host?.UpdateThread.Scheduler.Add(() => reset_scheduler.Add(disposalAction.Invoke));
+            if (host != null && host.TryGetTarget(out GameHost h))
+                h.UpdateThread.Scheduler.Add(() => reset_scheduler.Add(disposalAction.Invoke));
         }
 
         internal static void Reset(Vector2 size)

osu.Framework/Graphics/OpenGL/Textures/TextureGLSingle.cs

@@ -59,6 +59,9 @@ protected override void Dispose(bool isDisposing)
         {
             base.Dispose(isDisposing);
 
+            while (uploadQueue.TryDequeue(out TextureUpload u))
+                u.Dispose();
+
             GLWrapper.ScheduleDisposal(unload);
         }
 
@@ -67,9 +70,6 @@ protected override void Dispose(bool isDisposing)
         /// </summary>
         private void unload()
         {
-            while (uploadQueue.TryDequeue(out TextureUpload u))
-                u.Dispose();
-
             int disposableId = textureId;
 
             if (disposableId <= 0)

osu.Framework/Graphics/Visualisation/LogOverlay.cs

@@ -138,6 +138,12 @@ protected override void PopOut()
             enabled.Value = false;
             this.FadeOut(100);
         }
+
+        protected override void Dispose(bool isDisposing)
+        {
+            base.Dispose(isDisposing);
+            Logger.NewEntry -= addEntry;
+        }
     }
 
     internal class DrawableLogEntry : Container

osu.Framework/Logging/Logger.cs

@@ -7,8 +7,10 @@
 using System.Globalization;
 using System.IO;
 using osu.Framework.Platform;
-using osu.Framework.Threading;
 using System.Linq;
+using System.Threading;
+using System.Threading.Tasks;
+using osu.Framework.Threading;
 
 namespace osu.Framework.Logging
 {
@@ -18,6 +20,7 @@ namespace osu.Framework.Logging
     public class Logger
     {
         private static readonly object static_sync_lock = new object();
+
         // separate locking object for flushing so that we don't lock too long on the staticSyncLock object, since we have to
         // hold this lock for the entire duration of the flush (waiting for I/O etc) before we can resume scheduling logs
         // but other operations like GetLogger(), ApplyFilters() etc. can still be executed while a flush is happening.
@@ -55,17 +58,8 @@ public class Logger
         /// </summary>
         public static Storage Storage
         {
-            private get
-            {
-                return storage;
-            }
-
-            set
-            {
-                storage = value ?? throw new ArgumentNullException(nameof(value));
-                lock (flush_sync_lock)
-                    backgroundScheduler.Enabled = true;
-            }
+            private get { return storage; }
+            set { storage = value ?? throw new ArgumentNullException(nameof(value)); }
         }
 
         /// <summary>
@@ -141,6 +135,7 @@ public static void Log(string message, LoggingTarget target = LoggingTarget.Runt
         {
             log(message, target, null, level);
         }
+
         /// <summary>
         /// Log an arbitrary string to the logger with the given name.
         /// </summary>
@@ -338,7 +333,7 @@ private void add(string message = @"", LogLevel level = LogLevel.Verbose, bool o
                 if (!Enabled)
                     return;
 
-                backgroundScheduler.Add(delegate
+                scheduler.Add(delegate
                 {
                     try
                     {
@@ -351,6 +346,8 @@ private void add(string message = @"", LogLevel level = LogLevel.Verbose, bool o
                     {
                     }
                 });
+
+                writer_idle.Reset();
             }
         }
 
@@ -371,7 +368,10 @@ private void add(string message = @"", LogLevel level = LogLevel.Verbose, bool o
         private void clear()
         {
             lock (flush_sync_lock)
-                backgroundScheduler.Add(() => Storage.Delete(Filename));
+            {
+                scheduler.Add(() => Storage.Delete(Filename));
+                writer_idle.Reset();
+            }
         }
 
         private bool headerAdded;
@@ -390,17 +390,36 @@ private void ensureHeader()
 
         private static readonly List<string> filters = new List<string>();
         private static readonly Dictionary<string, Logger> static_loggers = new Dictionary<string, Logger>();
-        private static ThreadedScheduler backgroundScheduler = new ThreadedScheduler(@"Logger", startEnabled: false);
+
+        private static readonly Scheduler scheduler = new Scheduler();
+
+        private static readonly ManualResetEvent writer_idle = new ManualResetEvent(true);
+
+        static Logger()
+        {
+            Task.Factory.StartNew(() =>
+            {
+                while (true)
+                {
+                    if ((Storage != null ? scheduler.Update() : 0) == 0)
+                        writer_idle.Set();
+                    Thread.Sleep(50);
+                }
+
+                // ReSharper disable once FunctionNeverReturns
+            }, TaskCreationOptions.LongRunning);
+        }
 
         /// <summary>
         /// Pause execution until all logger writes have completed and file handles have been closed.
+        /// This will also unbind all handlers bound to <see cref="NewEntry"/>.
         /// </summary>
         public static void Flush()
         {
             lock (flush_sync_lock)
             {
-                backgroundScheduler.Dispose();
-                backgroundScheduler = new ThreadedScheduler(@"Logger") { Enabled = storage != null };
+                writer_idle.WaitOne(500);
+                NewEntry = null;
             }
         }
     }
@@ -414,14 +433,17 @@ public class LogEntry
         /// The level for which the message was logged.
         /// </summary>
         public LogLevel Level;
+
         /// <summary>
         /// The target to which this message is being logged, or null if it is being logged to a custom named logger.
         /// </summary>
         public LoggingTarget? Target;
+
         /// <summary>
         /// The name of the logger to which this message is being logged, or null if it is being logged to a specific <see cref="LoggingTarget"/>.
         /// </summary>
         public string LoggerName;
+
         /// <summary>
         /// The message that was logged.
         /// </summary>
@@ -437,14 +459,17 @@ public enum LogLevel
         /// Log-level for debugging-related log-messages. This is the lowest level (highest verbosity). Please note that this will log input events, including keypresses when entering a password.
         /// </summary>
         Debug,
+
         /// <summary>
         /// Log-level for most log-messages. This is the second-lowest level (second-highest verbosity).
         /// </summary>
         Verbose,
+
         /// <summary>
         /// Log-level for important log-messages. This is the second-highest level (second-lowest verbosity).
         /// </summary>
         Important,
+
         /// <summary>
         /// Log-level for error messages. This is the highest level (lowest verbosity).
         /// </summary>
@@ -460,22 +485,27 @@ public enum LoggingTarget
         /// Logging target for general information. Everything logged with this target will not be written to a logfile.
         /// </summary>
         Information,
+
         /// <summary>
         /// Logging target for information about the runtime.
         /// </summary>
         Runtime,
+
         /// <summary>
         /// Logging target for network-related events.
         /// </summary>
         Network,
+
         /// <summary>
         /// Logging target for performance-related information.
         /// </summary>
         Performance,
+
         /// <summary>
         /// Logging target for information relevant to debugging.
         /// </summary>
         Debug,
+
         /// <summary>
         /// Logging target for database-related events.
         /// </summary>

osu.Framework/Platform/GameHost.cs

@@ -35,7 +35,9 @@ namespace osu.Framework.Platform
 {
     public abstract class GameHost : IIpcHost, IDisposable
     {
-        public GameWindow Window;
+        public GameWindow Window { get; protected set; }
+
+        private readonly Toolkit toolkit;
 
         private FrameworkDebugConfigManager debugConfig;
 
@@ -149,6 +151,8 @@ public double MaximumInactiveHz
 
         protected GameHost(string gameName = @"")
         {
+            toolkit = Toolkit.Init();
+
             AppDomain.CurrentDomain.UnhandledException += exceptionHandler;
 
             FileSafety.DeleteCleanupDirectory();
@@ -494,6 +498,10 @@ private void stopAllThreads()
                 if (!t.Thread.Join(thread_join_timeout))
                     Logger.Log($"Thread {t.Name} failed to exit in allocated time ({thread_join_timeout}ms).", LoggingTarget.Runtime, LogLevel.Important);
             });
+
+            // as the input thread isn't actually handled by a thread, the above join does not necessarily mean it has been completed to an exiting state.
+            while (!InputThread.Exited)
+                InputThread.RunUpdate();
         }
 
         private void window_KeyDown(object sender, KeyboardKeyEventArgs e)
@@ -637,11 +645,18 @@ protected virtual void Dispose(bool disposing)
             while (executionState > ExecutionState.Stopped)
                 Thread.Sleep(10);
 
+            AppDomain.CurrentDomain.UnhandledException -= exceptionHandler;
+
             Root?.Dispose();
+            Root = null;
 
             config?.Dispose();
             debugConfig?.Dispose();
 
+            Window?.Dispose();
+
+            toolkit?.Dispose();
+
             Logger.Flush();
         }
 

osu.Framework/Platform/TcpIpcProvider.cs

@@ -53,11 +53,9 @@ public async Task StartAsync()
                     {
                         await Task.Delay(10, token);
                         if (token.IsCancellationRequested)
-                        {
-                            listener.Stop();
                             return;
-                        }
                     }
+
                     using (var client = await listener.AcceptTcpClientAsync())
                     {
                         using (var stream = client.GetStream())
@@ -85,6 +83,16 @@ public async Task StartAsync()
             catch (TaskCanceledException)
             {
             }
+            finally
+            {
+                try
+                {
+                    listener.Stop();
+                }
+                catch
+                {
+                }
+            }
         }
 
         public async Task SendMessageAsync(IpcMessage message)

osu.Framework/Platform/Windows/WindowsGameHost.cs

@@ -25,17 +25,21 @@ internal WindowsGameHost(string gameName, bool bindIPC = false)
             timePeriod = new TimePeriod(1) { Active = true };
 
             Window = new WindowsGameWindow();
-            Window.WindowStateChanged += (sender, e) =>
-            {
-                if (Window.WindowState != OpenTK.WindowState.Minimized)
-                    OnActivated();
-                else
-                    OnDeactivated();
-            };
+            Window.WindowStateChanged += onWindowOnWindowStateChanged;
+        }
+
+        private void onWindowOnWindowStateChanged(object sender, EventArgs e)
+        {
+            if (Window.WindowState != OpenTK.WindowState.Minimized)
+                OnActivated();
+            else
+                OnDeactivated();
         }
 
         protected override void Dispose(bool isDisposing)
         {
+            Window.WindowStateChanged -= onWindowOnWindowStateChanged;
+
             timePeriod?.Dispose();
             base.Dispose(isDisposing);
         }