Merge pull request ppy#6152 from hwsmm/dispose-waveform-stream

Fix Waveform not disposing Stream
Susko3 · Feb 2, 2024 · 6ba6ac8 · 6ba6ac8
2 parents 77a0ccd + 6db44b4
commit 6ba6ac8
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 9 deletions.
diff --git a/osu.Framework.Benchmarks/BenchmarkWaveform.cs b/osu.Framework.Benchmarks/BenchmarkWaveform.cs
@@ -16,7 +16,7 @@ namespace osu.Framework.Benchmarks
     [MemoryDiagnoser]
     public class BenchmarkWaveform : BenchmarkTest
     {
-        private Stream data = null!;
+        private byte[] data = null!;
         private Waveform preloadedWaveform = null!;
 
         public override void SetUp()
@@ -27,11 +27,11 @@ public override void SetUp()
 
             var store = new NamespacedResourceStore<byte[]>(new DllResourceStore(typeof(FrameworkTestScene).Assembly), "Resources");
 
-            data = store.GetStream("Tracks/sample-track.mp3");
+            data = store.Get("Tracks/sample-track.mp3");
 
             Debug.Assert(data != null);
 
-            preloadedWaveform = new Waveform(data);
+            preloadedWaveform = new Waveform(new MemoryStream(data));
             // wait for load
             preloadedWaveform.GetPoints();
         }
@@ -40,7 +40,7 @@ public override void SetUp()
         [Test]
         public async Task TestCreate()
         {
-            var waveform = new Waveform(data);
+            var waveform = new Waveform(new MemoryStream(data));
 
             var originalPoints = await waveform.GetPointsAsync().ConfigureAwait(false);
             Debug.Assert(originalPoints.Length > 0);

diff --git a/osu.Framework/Audio/Track/Waveform.cs b/osu.Framework/Audio/Track/Waveform.cs
@@ -67,14 +67,20 @@ public class Waveform : IDisposable
 
         private readonly Task readTask;
 
-        private FileCallbacks? fileCallbacks;
+        private Stream? data;
 
         /// <summary>
         /// Constructs a new <see cref="Waveform"/> from provided audio data.
         /// </summary>
-        /// <param name="data">The sample data stream. If null, an empty waveform is constructed.</param>
+        /// <param name="data">
+        /// The sample data stream.
+        /// The <see cref="Waveform"/> will take ownership of this stream and dispose it when done reading track data.
+        /// If null, an empty waveform is constructed.
+        /// </param>
         public Waveform(Stream? data)
         {
+            this.data = data;
+
             readTask = Task.Run(() =>
             {
                 if (data == null)
@@ -87,7 +93,7 @@ public Waveform(Stream? data)
                     return;
                 }
 
-                fileCallbacks = new FileCallbacks(new DataStreamFileProcedures(data));
+                FileCallbacks fileCallbacks = new FileCallbacks(new DataStreamFileProcedures(data));
 
                 const int bytes_per_sample = 4;
 
@@ -186,6 +192,11 @@ public Waveform(Stream? data)
                 finally
                 {
                     Bass.StreamFree(decodeStream);
+                    fileCallbacks.Dispose();
+
+                    data.Dispose();
+                    this.data = data = null;
+
                     if (sampleBuffer != null)
                         ArrayPool<float>.Shared.Return(sampleBuffer);
                 }
@@ -350,8 +361,9 @@ protected virtual void Dispose(bool disposing)
             cancelSource.Dispose();
             points = Array.Empty<Point>();
 
-            fileCallbacks?.Dispose();
-            fileCallbacks = null;
+            // Try disposing the stream again in case the task was not started.
+            data?.Dispose();
+            data = null;
         }
 
         #endregion